async_redux icon indicating copy to clipboard operation
async_redux copied to clipboard

Issue with viewmodel comparison

Open guy-plentific opened this issue 3 years ago • 0 comments

We have noticed issues when comparing viewmodels for equality and it is affecting testing. The below code demonstrates the outcome of comparisons:

import 'package:async_redux/async_redux.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter_test/flutter_test.dart';

class TestVm extends Vm {
  TestVm({
    required this.isLoading,
    required this.numbers,
  }) : super(equals: [
          isLoading,
          numbers,
        ]);

  final bool isLoading;
  final List<int> numbers;
}

void main() {
  test('xyz', () {
    final vmA = TestVm(isLoading: true, numbers: [1, 2, 3]);
    final vmB = TestVm(isLoading: true, numbers: [1, 2, 3]);


    expect([1, 2, 3] == [1, 2, 3], true); // test 1 - fails
    expect(listEquals([1, 2, 3], [1, 2, 3]), true); // test 2 - passes
    expect(vmA, vmB); // test 3 - fails
  });
}

We believe the reason for this failure lies in the equality checking in the Vm class. The below code is from view_model.dart (async_redux 15.0.0)

bool _listEquals<T>(List<T>? list1, List<T>? list2) {
    if (list1 == null) return list2 == null;
    if (list2 == null || list1.length != list2.length) return false;
    if (identical(list1, list2)) return true;
    for (int index = 0; index < list1.length; index++) {
      var item1 = list1[index];
      var item2 = list2[index];

      if ((item1 is VmEquals<T>) &&
          (item2 is VmEquals<T>) //
          &&
          !item1.vmEquals(item2)) return false;

      if (item1 != item2) return false;
    }
    return true;
  }

In the line if (item1 != item2) return false; we are returning false here if the items are lists. This is because we are checking list equality like we did in test 1, rather than using the listEquals of test 2. A potential fix may be to check if the items are lists (or iterables) and if so use the listEquals method instead.

What are your thoughts?

guy-plentific avatar Aug 18 '22 15:08 guy-plentific