Helper for sorting nested nullable properties

[GoogleCodeExporter]

I have been implementing `Ordering`s/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.

Original issue reported on code.google.com by jens.ran...@tink.se on 10 Jul 2014 at 12:25

[GoogleCodeExporter]

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<MyProperty> comparator = comparing(MyProperty::getSubProperty,
     nullsFirst(comparing(SubProperty::getSubSubProperty,
       nullsFirst(comparing(SubSubProperty::value)))));

Original comment by wasserman.louis on 10 Jul 2014 at 3:55

[GoogleCodeExporter]

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:
 - to minimize repetition of the values being compared. They are only needed at instantiation and will thus minimize repetition (which is similar goad with other comparison helpers in Guava).
 - easily check if any of the values are null.
 - easily sort based on nulls.

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.

Original comment by jens.ran...@tink.se on 12 Jul 2014 at 9:08

[GoogleCodeExporter]

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<MyProperty> 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.

Original comment by cpov...@google.com on 16 Jul 2014 at 6:29

[GoogleCodeExporter]

This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:08

• Added labels: MigratedToGitHub

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:17

• Changed state: Migrated

[GoogleCodeExporter]

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:07

• Added labels: Restrict-AddIssueComment-Commit