(Nested) List Diff

[pdewan]

This is lovely tool and you have set up a nice hierarchical structure for diffing.

Below is my first example and its output, which raises several issues:

1. Not sure about the log4 messages, I assume googling will address it.
2. It would be nice to distinguish Lists from collections, and have property paths name indices rather than elements. This seems like an easy addition, all you have to do is call indexOf given the element and the list. Instead of Collection path element now you would have a List path element.
3. Most important, the nested list output goes against intuition. If a first level diff is being on the list, then we should be told that the first element (a list) of the nested base list has been replaced by the first element of the nested working list. If a second level diff is being done, then the output should be the same as the output of the flat list (except for the paths). Instead we have a second level diff in which all elements of the base list are considered deleted and the elements of the working list are considered added.
4. I thought of trying to extend your tool but lots of classes (PropertyPath, Collection element) are final and there is no way to add your own diff factories without changing your code, which I did not want to do, as I want to keep getting your updates.

Still this a wonderful tool and the problems above can be addressed with a post processing of the output.

Program:
public static void main (String[] args) {
String[] stringsWorking = {"b1", "w1", "w2", "b4"};
String[] stringsBase = {"b1", "b2", "b3", "b4"};
List listWorking = Arrays.asList(stringsWorking);
List listBase = Arrays.asList(stringsBase);
List nestedListWorking = new ArrayList();
List nestedListBase = new ArrayList();
nestedListWorking.add(listWorking);
nestedListBase.add(listBase);
ObjectDiffer objectDiffer = ObjectDifferFactory.getInstance();
Node listRoot = objectDiffer.compare(listWorking, listBase);
listRoot.visit(new PrintingVisitor(listWorking, listBase));
Node nestedListRoot = objectDiffer.compare(nestedListWorking, nestedListBase);
nestedListRoot.visit(new PrintingVisitor(nestedListWorking, nestedListBase));
}

Output:
log4j:WARN No appenders could be found for logger (de.danielbechler.util.Classes).
log4j:WARN Please initialize the log4j system properly.
Property at path '/[w1]' has been added => [ w1 ]
Property at path '/[w2]' has been added => [ w2 ]
Property at path '/[b2]' with value [ b2 ] has been removed
Property at path '/[b3]' with value [ b3 ] has been removed
Property at path '/[[b1, w1, w2, b4]][b1]' has been added => [ b1 ]
Property at path '/[[b1, w1, w2, b4]][w1]' has been added => [ w1 ]
Property at path '/[[b1, w1, w2, b4]][w2]' has been added => [ w2 ]
Property at path '/[[b1, w1, w2, b4]][b4]' has been added => [ b4 ]
Property at path '/[[b1, b2, b3, b4]][b1]' with value [ b1 ] has been removed
Property at path '/[[b1, b2, b3, b4]][b2]' with value [ b2 ] has been removed
Property at path '/[[b1, b2, b3, b4]][b3]' with value [ b3 ] has been removed
Property at path '/[[b1, b2, b3, b4]][b4]' with value [ b4 ] has been removed

[SQiShER]

This is lovely tool and you have set up a nice hierarchical structure for diffing.

Thanks! It's great to hear that!

Not sure about the log4 messages, I assume googling will address it.

That's just log4j in your classpath complaining about a missing configuration file.

It would be nice to distinguish Lists from collections, and have property paths name indices rather than elements. This seems like an easy addition, all you have to do is call indexOf given the element and the list. Instead of Collection path element now you would have a List path element.

That is a good point. Treating Lists differently would certainly make some things easier. To be honest, Collection/List-diffing is not fully implemented yet. There are several open issues with the current implementation. One of the major ones being that Lists are currently treated like Collections and their diffs currently don't properly reflect cases in which the same object is multiple times in the same List. I think part of the solution will be to have a special ListDiffer. In this case it will be no problem to use another representation.

Most important, the nested list output goes against intuition. If a first level diff is being on the list, then we should be told that the first element (a list) of the nested base list has been replaced by the first element of the nested working list. If a second level diff is being done, then the output should be the same as the output of the flat list (except for the paths). Instead we have a second level diff in which all elements of the base list are considered deleted and the elements of the working list are considered added.

I totally agree. This also comes back to the open issues I mentioned above. Ideally I would prefer a diff stating that the elements have been moved to different positions in the List. I'll make this behavior a part of the feature specification to make sure that the final implementation works more intuitively.

I thought of trying to extend your tool but lots of classes (PropertyPath, Collection element) are final and there is no way to add your own diff factories without changing your code, which I did not want to do, as I want to keep getting your updates.

That's a decision I made to protect people from making changes the to library that could potentially break with every update. In the current state where more and more people start using it, many people give great feedback and come up with interesting feature requests, that sometimes force me to change huge parts of the code base, in order to implement them properly. Keeping the public API as small as possible gives me the chance to do this without upsetting people by breaking their code. Once the code base becomes more stable, I'm planning to add some hooks, to inject own implementations for certain things.

However, I'm always happy to see people fork the library, make the changes they need and send me a pull request. Every helping hand is welcome!

Still this a wonderful tool and the problems above can be addressed with a post processing of the output.

Or you could just write your own Visitor and generate the output the way you need! If you have any questions on how to do that, feel free to ask. I'd be happy to answer and could use the questions as inspiration for a more elaborate Wiki article about Visitors.

[pdewan]

Thanks Daniel for your thorough answer!!

On Fri, May 10, 2013 at 12:51 PM, Daniel Bechler
notifications@github.comwrote:

This is lovely tool and you have set up a nice hierarchical structure for
diffing.

Thanks! It's great to hear that!

Not sure about the log4 messages, I assume googling will address it.

That's just log4j in your classpath complaining about a missing
configuration file.

It would be nice to distinguish Lists from collections, and have property
paths name indices rather than elements. This seems like an easy addition,
all you have to do is call indexOf given the element and the list. Instead
of Collection path element now you would have a List path element.

That is a good point. Treating Lists differently would certainly make some
things easier. To be honest, Collection/List-diffing is not fully
implemented yet. There are several open issues with the current
implementation. One of the major ones being that Lists are currently
treated like Collections and their diffs currently don't properly reflect
cases in which the same object is multiple times in the same List. I think
part of the solution will be to have a special ListDiffer. In this case
it will be no problem to use another representation.

Most important, the nested list output goes against intuition. If a first
level diff is being on the list, then we should be told that the first
element (a list) of the nested base list has been replaced by the first
element of the nested working list. If a second level diff is being done,
then the output should be the same as the output of the flat list (except
for the paths). Instead we have a second level diff in which all elements
of the base list are considered deleted and the elements of the working
list are considered added.

I totally agree. This also comes back to the open issues I mentioned
above. Ideally I would prefer a diff stating that the elements have been
moved to different positions in the List. I'll make this behavior a part of
the feature specification to make sure that the final implementation works
more intuitively.

I thought of trying to extend your tool but lots of classes (PropertyPath,
Collection element) are final and there is no way to add your own diff
factories without changing your code, which I did not want to do, as I want
to keep getting your updates.

That's a decision I made to protect people from making changes the to
library that could potentially break with every update. In the current
state where more and more people start using it, many people give great
feedback and come up with interesting feature requests, that sometimes
force me to change huge parts of the code base, in order to implement them
properly. Keeping the public API as small as possible gives me the chance
to do this without upsetting people by breaking their code.

However, I'm always happy to see people fork the library, make the changes
they need and send me a pull request. Every helping hand is welcome!

Still this a wonderful tool and the problems above can be addressed with a
post processing of the output.

Or you could just write your own Visitorhttps://github.com/SQiShER/java-object-diff/wiki/Getting-Started#visitorsand generate the output the way you need! If you have any questions on how
to do that, feel free to ask. I'd be happy to answer and could use the
questions as inspiration for a more elaborate Wiki article about Visitors.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/60#issuecomment-17731438
.

[SQiShER]

Now that the library is much more extensible, this will be addressed, once we finally start working towards more advanced list diffing. I'm closing this issue for now, since it will be referenced from a new issue.