migrator / guava-libraries-2

Guava: Google Core Libraries for Java 1.6+
0 stars 0 forks source link

Helper for sorting nested nullable properties #41

Open migrator opened 9 years ago

migrator commented 9 years ago

I have been implementing Orderings/Comparators/Comparables of nested properties and everytime I do this I get tired of all the null checks to handle ordering of nullable nested propertys. Currently Guava does not help me here.

Example code I'm tired of:

public int compare(MyProperty left, MyProperty right) { if (left.getSubProperty() == null || right.getSubProperty()) { return 0; } if (left.getSubProperty().getSubSubProperty() == null || right.getSubProperty().getSubSubProperty() == null) { return 0; } return ComparisonChain.start().compare(left.getSubProperty().getSubSubProperty().value(), right.getSubProperty().getSubSubProperty().value()).result(); }

If I'd like to order by null value it gets even worse:

public int compare(MyProperty left, MyProperty right) { if (left.getSubProperty() == null || right.getSubProperty()) { return ComparisonChain.start().compare(left.getSubProperty(), right.getSubProperty()).result(); } if (left.getSubProperty().getSubSubProperty() == null || right.getSubProperty().getSubSubProperty() == null) { return ComparisonChain.start().compare(left.getSubProperty().getSubSubProperty(), right.getSubProperty().getSubSubProperty()).result(); } return ComparisonChain.start().compare(left.getSubProperty().getSubSubProperty().value(), right.getSubProperty().getSubSubProperty().value()).result(); }

Two proposed solutions: https://gist.github.com/JensRantil/3063132784a0945dc6f0

I am leaning towards Example2 and have started using it in my code. I'd love to hear about other solutions.

relevance: 2

migrator commented 9 years ago

summary: Not Defined

FWIW, your first Comparator doesn't satisfy the contract: it's not transitive, since something with a getSubProperty() == null compares as equal to everything, but not everything is equal to everything else.

As far as your approaches go, exceptions of any kind are usually a nasty performance killer, so I'd be concerned about approach 2. Approach 1 is somewhat interesting, but a bit difficult to follow.

In Java 8, you could do something quite short and neat here:

import static java.util.Comparator.*;

Comparator comparator = comparing(MyProperty::getSubProperty, nullsFirst(comparing(SubProperty::getSubSubProperty, nullsFirst(comparing(SubSubProperty::value)))));

status Not Defined creator: wasserman.louis created at: Jul 10, 2014

migrator commented 9 years ago

summary: Not Defined

Hey,

Thanks for feedback! As for the non-transitiveness it was simply a typo (and cope-pasty error). Apologize for that.

I did not know about exceptions being a performance killer. That's unfortunate, because I really liked that solution!

The idea with example 1 is to have a helper class with the following purposes:

Thinking about this I also realized there's a third solution that does not require any of my helper classes/methods:

ComparisonChain chain = ComparisonChain.start(); try { chain = chain.compare(left.getSubProperty(), right.getSubProperty()); chain = chain.compare(left.getSubProperty().getSubSubProperty(), right.getSubProperty().getSubSubProperty()); chain = chain.compare(left.getSubProperty().getSubSubProperty().value(), right.getSubProperty().getSubSubProperty().value()); catch (NullPointerException e) { return chain.result(); }

This obviously would use exceptions, too, but [1] hints that exceptions thrown and caught within the same method is performant and effectively handled by a goto[2]. Then there's the debate of catching NullpointerExceptions...

I'd love to hear your opinion and whether there's an Guava-idiomatic way of doing these kind of comparisons.

[1] http://stackoverflow.com/a/299315/260805 [2] ..although, I haven't benchmarked this myself.

status Not Defined creator: jens.ran...@tink.se created at: Jul 12, 2014

migrator commented 9 years ago

summary: Not Defined

To reduce duplication, could you extract a method that pulls out the sub-sub value (or null)? Then you could call it on each input and compare the values. Or, at that point, you could use Ordering.onResultOf:

private static Ordering ORDER = Ordering.natural().nullsFirst().onResultOf( new Function<MyProperty, SubSubProperty>() { @Override public SubSubProperty getSubSubProperty(MyProperty property) { return (property.getSubProperty() == null || property.getSubProperty().getSubSubProperty() == null) ? null : property.getSubProperty().getSubSubProperty().value(); } };

There's less duplication, but there's still a bunch of boilerplate. I'd be happy to hear better suggestions, but I'm not sure how much better we'll do without Louis's Java 8 suggestion.

status Not Defined creator: cpov...@google.com created at: Jul 16, 2014