dart-archive / observe

Support for marking objects as observable, and getting notifications when those objects are mutated
https://pub.dartlang.org/packages/observe
BSD 3-Clause "New" or "Revised" License
13 stars 6 forks source link

ObservableList : sorting 2 Element lists in ReactionFn = infinite loop #79

Closed eRaz0rHead closed 8 years ago

eRaz0rHead commented 8 years ago

Calling list.sort() within a change listener generally works ok (there are no doubt better solutions than sorting on change, but that's not the point) -- the ObservableList notices that the list hasn't changed after sorting and doesn't emit another change.

But when the list is exactly 2 elements, ObservableList emits the same changeRecord infinitely.

I suspect this might be fixed in []= (copied below) by checking that oldValue != value as well as _hasListObservers, but that might have further reaching effects in the list_diff code. The bug might also stem from an optimization within list_diff, but I haven't looked there yet.

@reflectable void operator []=(int index, E value) {
    var oldValue = _list[index];
    if (_hasListObservers) {  // && oldValue != value?
      _recordChange(new ListChangeRecord(this, index, addedCount: 1,
          removed: [oldValue]));
    }
    _list[index] = value;
  }

Here's an example test :

int MAX_LOOPS = 100;
int loopCounter = 0;

Function reSort(list) => (changes) {
      // Uncomment the following line to ensure the test can exit.
      // if (loopCounter++ > MAX_LOOPS) throw 'MAX LOOPS EXCEEDED';
      print('changes $changes');
      list.sort();
    };

void main() {
  // This one works fine.
  test('Sorting three items', () {
    ObservableList list = toObservable([3, 2, 1]);
    list.listChanges.listen(reSort(list));
    list.sort();
    expect(list, [1, 2, 3]);
  });

  // This one will loop infinitely unless you un-comment the throws line in [resort].
  test('Sorting two items', () {
    ObservableList list = toObservable([3, 1]);
    list.listChanges.listen(reSort(list));
    list.sort();
    expect(list, [1, 3]);
  });
}