knaeckeKami / diffutil.dart

Apache License 2.0
27 stars 4 forks source link

DataChange not dispatched when value in List changes #13

Open Robbendebiene opened 2 years ago

Robbendebiene commented 2 years ago

Hi, I'm trying to understanding the change callback on the DataDiffUpdate class.

Why does

diffutil.calculateListDiff([1, 2, 3], [1, 0, 3]).getUpdatesWithData();

return

      [
        DataRemove(position: 1, data: 2),
        DataInsert(position: 1, data: 0)
      ];

I would have assumed a single DataChange.

Can you please explain when a DataChange will be created?

knaeckeKami commented 2 years ago

Yes. DataChange is related to identity vs. equality. It is emitted, when an item represents the same item as before but with different data.

The goal of this was to be able to use different animation when the data of an item in the list changes vs. when one item was removed and a new item was inserted the same position.

By default, when just using equality, it is never emitted.

For example, when you have a List of type DataObject that has a unique id, you could write a DiffDelegate like this:

class DataObjectListDiff extends diffutil.ListDiffDelegate<DataObject> {
  DataObjectListDiff(List<DataObject> oldList, List<DataObject> newList)
      : super(oldList, newList);

  @override
  bool areContentsTheSame(int oldItemPosition, int newItemPosition) {
    return equalityChecker(oldList[oldItemPosition], newList[newItemPosition]);
  }

  @override
  bool areItemsTheSame(int oldItemPosition, int newItemPosition) {
    return oldList[oldItemPosition].id == newList[newItemPosition].id;
  }
}

And then call

 diffutil
          .calculateDiff<DataObject>(DataObjectListDiff(
              [DataObject(id: 1, payload: 0)], [DataObject(id: 1, payload: 1)]))
          .getUpdatesWithData();
Robbendebiene commented 2 years ago

Thank you, this is exactly what I was looking for.

To gain some benefit for others out of this issue, perhaps you could add the example you've provided above to the documentation? 😀

knaeckeKami commented 2 years ago

Yes, I put it on my todo list :)

giorgio79 commented 2 years ago

Wow great question! I just had this issue using this fantastic library. I am using it to compare lists of strings (tokenized texts)... I don't completely understand the reasoning between equality and identity. How would I go about with simple string lists? Perhaps if the comparison is between simple lists, change should be emitted by default perhaps?

diffutil.calculateListDiff(["Today", "was", "nice"], ["Today", "is", "nice"]).getUpdatesWithData();

giorgio79 commented 2 years ago

PS it is also unclear from the example how we can get changes for the basic example diffutil.calculateListDiff([1, 2, 3], [1, 0, 3]).getUpdatesWithData(); @Robbendebiene did you get it to work?

Extending like this does not work.

Perhaps we should have separate calls for Lists and Objects?

class DataObjectListDiff extends diffutil.ListDiffDelegate<String> {
  DataObjectListDiff(List<String> oldList, List<String> newList)
      : super(oldList, newList);

  @override
  bool areContentsTheSame(int oldItemPosition, int newItemPosition) {
    return equalityChecker(oldList[oldItemPosition], newList[newItemPosition]);
  }

  @override
  bool areItemsTheSame(int oldItemPosition, int newItemPosition) {
    return oldList[oldItemPosition] == newList[newItemPosition];
  }
}
knaeckeKami commented 2 years ago

The changes are really meant for cases when you have a meaningful identity.

Like if you have a User-Object with the content

{
  "id": 1
  "name" : "Joe"
}

and

{
  "id": 1
  "name" : "Peter"
}

It's easy to detect that that's the same user but with a different name. But with plain Strings, there is no identity.

If you don't have a field like id, you probably won't get acceptable results with this algorithm for detecting changes.

It might be possible the do a post-process step where inserts and removals are merged to change operations afterward - but it's not implemented as of yet.

But if you really just need strings, maybe check out Google's diff-match-patch library. It's similar to diffutil but focused purely on text-maybe this will help you.

giorgio79 commented 2 years ago

Ah ok. Changes for a simple list diff would have been nice, bit I guess I can convert a list of strings to a list of plain objects with an id... I saw the Google algo but I don't think it gives operations... The closest package is this one https://github.com/kpdecker/jsdiff

giorgio79 commented 2 years ago

PS if we could have an example with objects that would be fab :) https://github.com/knaeckeKami/diffutil.dart/blob/master/example/lib/main.dart

knaeckeKami commented 2 years ago

Thanks for the feedback. Yeah I'll update documentation and more examples soon.

knaeckeKami commented 2 years ago

Ah ok. Changes for a simple list diff would have been nice, bit I guess I can convert a list of strings to a list of plain objects with an id...

Yes, unfortunately I don't really see a way of implementing that. The algorithm, that most of these diff-libraries are based on, is Myer's algorithm and it does not support change operations per se- It's just that if you have a unique ID, this can be added pretty easily.

For general support without IDs the algorithm would need to be reworked (not within by abilities, to be honest) or a post-processing step, similar to batching, would be necessary. The post-processing step might be feasible but I don't have the time at the moment to implement it. But PR's always welcome.

Robbendebiene commented 2 years ago

Actually I got it to work without an id, though I'm not 100% sure how safe it is. At first I only compared the indexes, but this wasn't enough if one deletes an item in between, because this will then be treated as an update/change while the last item in the list will be treated as the deletion. Therefore I've added an additional object equality check (there is no special equality function for the objects in place). Still I'm not quite sure why this additional check works, because I'm not familiar on how the algorithm makes use of this function.

class MYOBJECTListDiff extends ListDiffDelegate<MYOBJECT> {

   MYOBJECTListDiff(List<MYOBJECT> oldList, List<MYOBJECT> newList)
      : super(oldList, newList);

  @override
  bool areContentsTheSame(int oldItemPosition, int newItemPosition) {
    return oldList[oldItemPosition].name == newList[newItemPosition].name;
  }

  @override
  bool areItemsTheSame(int oldItemPosition, int newItemPosition) {
    return oldItemPosition == newItemPosition || oldList[oldItemPosition] == newList[newItemPosition];
  }
}
knaeckeKami commented 2 years ago

I think this might work, yes.

It's important to understand that areItemsTheSame is called first, areContentsTheSame is only called when areItemsTheSame returns true (default implementation).

If you have an implementation where areItemsTheSame returns false but areContentsTheSame returns true, you might get a result that is not minimal (the result list might be longer than needed).