Closed GoogleCodeExporter closed 9 years ago
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
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
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
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
Original comment by cgdecker@google.com
on 1 Nov 2014 at 4:17
Original comment by cgdecker@google.com
on 3 Nov 2014 at 9:07
Original issue reported on code.google.com by
jens.ran...@tink.se
on 10 Jul 2014 at 12:25