Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RangeError (index): Invalid value: Not in inclusive range #8

Closed
realkalash opened this issue Sep 25, 2021 · 4 comments
Closed

RangeError (index): Invalid value: Not in inclusive range #8

realkalash opened this issue Sep 25, 2021 · 4 comments
Assignees

Comments

@realkalash
Copy link

realkalash commented Sep 25, 2021

Help please. When i try to use a simple solution i get error. Maybe I am using diffUtil incorrectly somehow?...

My code:

    final List<int> oldList = [10, 20,30];
    final List<int> newList = [30,40];

    final diffResult = calculateListDiff(
      oldList,
      newList,
    );

    for (var update in diffResult.getUpdates(batch: false)) {
      update.when(
        insert: (position, count) {
          print("inserted count:$count on pos $position. Its in newList: ${newList[position]}");
          return;
        },
        remove: (position, count) {
          print("removed on pos $position. Its in oldList: ${oldList[position]}");
          return;
        },
        change: (position, payload) {
          return;
        },
        move: (from, to) {
          print("move in oldList element $from to $to");
          return;
        },
      );
    }

Error i get:

dart:core                                              List.[]
test\widget_test.dart 141:81                           main.<fn>.<fn>
package:diffutil_dart/src/model/diffupdate.dart 59:18  Insert.when
test\widget_test.dart 138:14                           main.<fn>

RangeError (index): Invalid value: Not in inclusive range 0..1: 3

What i expected:

inserted count: 1 on pos 4 Its in oldList: 40
removed on pos 0. Its in oldList: 10
removed on pos 1. Its in oldList: 20
@knaeckeKami
Copy link
Owner

I elaborated here how diffutil works.

The updates are relative to the old list.
The old list is transformed to the list news by inserting one item on position 3 and then removing one item on position 1 and one on position 0.

[10, 20, 30] -> insert on position 3
[10, 20, 30, 40]-> remove on position 1
[10, 30, 40] -> remove on position 0
[30, 40]

Currently, there is no direct way to get the item that was inserted or removed.

You can check out the branch data_inserts where I added this, but I didn't test it thoroughly yet and the API is still a bit rough and this is a breaking change

@pawelsa
Copy link

pawelsa commented Oct 5, 2021

I have a similar issue to this one, so I think the comment is relevant.

I've prepared a few tests, as I wanted to make sure if my code is correct or not. In those tests, the last 4 do not pass.

test('should insert last two items of new list', () {
      final newList = [1, 2, 3, 4];
      final oldList = [1, 2];

      final diff = obtainDiff(
        delegate: _IntDiffDelegate(oldList, newList),
        newList: newList,
        oldList: oldList,
      );

      expect(diff.toInsert, [3, 4]);
      expect(diff.toUpdate, []);
      expect(diff.toRemove, []);
    });

    test('should insert first two items of new list', () {
      final newList = [1, 2, 3, 4];
      final oldList = [3, 4];

      final diff = obtainDiff(
        delegate: _IntDiffDelegate(oldList, newList),
        newList: newList,
        oldList: oldList,
      );

      expect(diff.toInsert, [1, 2]);
      expect(diff.toUpdate, []);
      expect(diff.toRemove, []);});

    test('should insert last item of new list', () {
      final newList = [1, 2, 3, 4];
      final oldList = [1, 2, 3];

      final diff = obtainDiff(
        delegate: _IntDiffDelegate(oldList, newList),
        newList: newList,
        oldList: oldList,
      );

      expect(diff.toInsert, [4]);
      expect(diff.toUpdate, []);
      expect(diff.toRemove, []);});

    test('should insert first item of new list', () {
      final newList = [1, 2, 3, 4];
      final oldList = [2, 3, 4];

      final diff = obtainDiff(
        delegate: _IntDiffDelegate(oldList, newList),
        newList: newList,
        oldList: oldList,
      );

      expect(diff.toInsert, [1]);
      expect(diff.toUpdate, []);
      expect(diff.toRemove, []);});

    test('should insert all new items from new list', () {
      final newList = [1, 2, 3, 4];
      final oldList = <int>[];

      final diff = obtainDiff(
        delegate: _IntDiffDelegate(oldList, newList),
        newList: newList,
        oldList: oldList,
      );

      expect(diff.toInsert, [1, 2, 3, 4]);
      expect(diff.toUpdate, []);
      expect(diff.toRemove, []);});

    test('should insert new item from middle of new list', () {
      final newList = [1, 2, 3, 4];
      final oldList = [1, 2, 4];

      final diff = obtainDiff(
        delegate: _IntDiffDelegate(oldList, newList),
        newList: newList,
        oldList: oldList,
      );

      expect(diff.toInsert, [3]);
      expect(diff.toUpdate, []);
      expect(diff.toRemove, []);});

  test('should remove last two items', (){
    final newList = [1, 2 ];
    final oldList = [1, 2, 3, 4];

    final diff = obtainDiff(
      delegate: _IntDiffDelegate(oldList, newList),
      newList: newList,
      oldList: oldList,
    );

    expect(diff.toInsert, []);
    expect(diff.toUpdate, []);
    expect(diff.toRemove, [3, 4]);
  });

  test('should remove last item', (){

    final newList = [1, 2, 3];
    final oldList = [1, 2, 3, 4];

    final diff = obtainDiff(
      delegate: _IntDiffDelegate(oldList, newList),
      newList: newList,
      oldList: oldList,
    );

    expect(diff.toInsert, []);
    expect(diff.toUpdate, []);
    expect(diff.toRemove, [4]);
  });

  test('should remove first two items', (){

    final newList = [3, 4];
    final oldList = [1, 2, 3, 4];

    final diff = obtainDiff(
      delegate: _IntDiffDelegate(oldList, newList),
      newList: newList,
      oldList: oldList,
    );

    expect(diff.toInsert, []);
    expect(diff.toUpdate, []);
    expect(diff.toRemove, [1, 2]);
  });

  test('should remove first item', (){
    final newList = [2, 3, 4];
    final oldList = [1, 2, 3, 4];

    final diff = obtainDiff(
      delegate: _IntDiffDelegate(oldList, newList),
      newList: newList,
      oldList: oldList,
    );

    expect(diff.toInsert, []);
    expect(diff.toUpdate, []);
    expect(diff.toRemove, [1]);
  });

  test('should remove middle item', (){
    final newList = [1, 2, 4];
    final oldList = [1, 2, 3, 4];

    final diff = obtainDiff(
      delegate: _IntDiffDelegate(oldList, newList),
      newList: newList,
      oldList: oldList,
    );

    expect(diff.toInsert, []);
    expect(diff.toUpdate, []);
    expect(diff.toRemove, [3]);
  });

  test('should insert in the middle and one at the end', (){
    final newList = [1, 2, 3, 4];
    final oldList = [1, 3];

    final diff = obtainDiff(
      delegate: _IntDiffDelegate(oldList, newList),
      newList: newList,
      oldList: oldList,
    );

    expect(diff.toInsert, [2, 4]);
    expect(diff.toUpdate, []);
    expect(diff.toRemove, []);
  });

  test('should insert in the middle and two at the end', (){
    final newList = [1, 2, 3, 4, 5];
    final oldList = [1, 3];

    final diff = obtainDiff(
      delegate: _IntDiffDelegate(oldList, newList),
      newList: newList,
      oldList: oldList,
    );

    expect(diff.toInsert, [2, 4, 5]);
    expect(diff.toUpdate, []);
    expect(diff.toRemove, []);
  });

  test('should insert in the middle and one at the start', (){
    final newList = [1, 2, 3, 4];
    final oldList = [2, 4];

    final diff = obtainDiff(
      delegate: _IntDiffDelegate(oldList, newList),
      newList: newList,
      oldList: oldList,
    );

    expect(diff.toInsert, [1, 3]);
    expect(diff.toUpdate, []);
    expect(diff.toRemove, []);
  });

  test('should insert int the middle and two at the start', (){
    final newList = [1, 2, 3, 4, 5];
    final oldList = [3, 5];

    final diff = obtainDiff(
      delegate: _IntDiffDelegate(oldList, newList),
      newList: newList,
      oldList: oldList,
    );

    expect(diff.toInsert, [1, 2, 4]);
    expect(diff.toUpdate, []);
    expect(diff.toRemove, []);
  });

where obtainDiff is my collection "mechanism" which looks like this:

DiffResult<T> obtainDiff<T>({
  @required DiffDelegate delegate,
  @required List<T> newList,
  @required List<T> oldList,
  bool detectMoves = true,
}) {
  final diff = calculateDiff(
    delegate,
    detectMoves: detectMoves,
  );
  final toInsert = <T>[];
  final toUpdate = <T>[];
  final toRemove = <T>[];
  for (var update in diff.getUpdates(batch: true)) {
    update.when(
      insert: (pos, count) {
        toInsert.addAll(newList.sublist(pos, pos + count));
      },
      change: (pos, payload) {
        toUpdate.add(payload);
      },
      remove: (pos, count) {
        toRemove.addAll(
          oldList.sublist(pos, (pos + count).clamp(pos + 1, oldList.length)));
      },
      move: (from, to) {
        print("from: $from, to: $to");
      },
    );
  }
  return DiffResult(toInsert, toUpdate, toRemove);
}

I've observed that in those 4 tests the insert method gives wrong values as it says "from position 1, two items should be inserted" instead of "from position 1 insert one item and from position 3 insert one item", so somehow it skips the item in the middle. In my current project, I am not on Flutter 2.0 so I cannot check the branch with proof of concept, maybe later this week I will do it. I think those tests might be helpful with further development.

@knaeckeKami
Copy link
Owner

knaeckeKami commented Oct 5, 2021

I see.

Yeah, with the current API, it is very hard to actually get the concrete items that have been removed or inserted.
You won't be able the calculate them like this. (because the order of the operation matters: the (pos, count) parameters assume that all previous operations have already been applied to the old list).

If you provide a minimal executable example I could look further into this, but I think that the result is correct, but diffutil just does not do what you need at the moment.
I designed to API to play nicely with SliverAnimatedListState, where you don't need to know which items actually have been inserted or not, you just need to know which positions should be animated.
See https://github.com/flyerhq/flutter_chat_ui/blob/a83576089c18e70a0503c0e70d4fbc86e0d21d68/lib/src/widgets/chat_list.dart#L83

What you need are is possible with experimental changes I made in the branch data_inserts.

Note that the implementation is pretty hacky at the moment (the changes only work with implementations of ListDiffDelegate) and just a proof of concept.

If I were to actually release this feature, I would probably provide separate methods for users that need this to avoid breaking changes.

But if you are able to check out this branch, I am happy to hear your feedback.

@knaeckeKami knaeckeKami self-assigned this Oct 5, 2021
@knaeckeKami
Copy link
Owner

I implemented this in a cleaner way and merged it on master.
Please check it out.

On how to use it, see https://github.com/knaeckeKami/diffutil.dart#updates-with-data

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants