How to simplify a null-safe compareTo() implementation?
Using Java 8:
private static Comparator<String> nullSafeStringComparator = Comparator
.nullsFirst(String::compareToIgnoreCase);
private static Comparator<Metadata> metadataComparator = Comparator
.comparing(Metadata::getName, nullSafeStringComparator)
.thenComparing(Metadata::getValue, nullSafeStringComparator);
public int compareTo(Metadata that) {
return metadataComparator.compare(this, that);
}
You can simply use Apache Commons Lang:
result = ObjectUtils.compare(firstComparable, secondComparable)
See the bottom of this answer for updated (2013) solution using Guava.
This is what I ultimately went with. It turned out we already had a utility method for null-safe String comparison, so the simplest solution was to make use of that. (It's a big codebase; easy to miss this kind of thing :)
public int compareTo(Metadata other) {
int result = StringUtils.compare(this.getName(), other.getName(), true);
if (result != 0) {
return result;
}
return StringUtils.compare(this.getValue(), other.getValue(), true);
}
This is how the helper is defined (it's overloaded so that you can also define whether nulls come first or last, if you want):
public static int compare(String s1, String s2, boolean ignoreCase) { ... }
So this is essentially the same as Eddie's answer (although I wouldn't call a static helper method a comparator) and that of uzhin too.
Anyway, in general, I would have strongly favoured Patrick's solution, as I think it's a good practice to use established libraries whenever possible. (Know and use the libraries as Josh Bloch says.) But in this case that would not have yielded the cleanest, simplest code.
Edit (2009): Apache Commons Collections version
Actually, here's a way to make the solution based on Apache Commons NullComparator
simpler. Combine it with the case-insensitive Comparator
provided in String
class:
public static final Comparator<String> NULL_SAFE_COMPARATOR
= new NullComparator(String.CASE_INSENSITIVE_ORDER);
@Override
public int compareTo(Metadata other) {
int result = NULL_SAFE_COMPARATOR.compare(this.name, other.name);
if (result != 0) {
return result;
}
return NULL_SAFE_COMPARATOR.compare(this.value, other.value);
}
Now this is pretty elegant, I think. (Just one small issue remains: the Commons NullComparator
doesn't support generics, so there's an unchecked assignment.)
Update (2013): Guava version
Nearly 5 years later, here's how I'd tackle my original question. If coding in Java, I would (of course) be using Guava. (And quite certainly not Apache Commons.)
Put this constant somewhere, e.g. in "StringUtils" class:
public static final Ordering<String> CASE_INSENSITIVE_NULL_SAFE_ORDER =
Ordering.from(String.CASE_INSENSITIVE_ORDER).nullsLast(); // or nullsFirst()
Then, in public class Metadata implements Comparable<Metadata>
:
@Override
public int compareTo(Metadata other) {
int result = CASE_INSENSITIVE_NULL_SAFE_ORDER.compare(this.name, other.name);
if (result != 0) {
return result;
}
return CASE_INSENSITIVE_NULL_SAFE_ORDER.compare(this.value, other.value);
}
Of course, this is nearly identical to the Apache Commons version (both use
JDK's CASE_INSENSITIVE_ORDER), the use of nullsLast()
being the only Guava-specific thing. This version is preferable simply because Guava is preferable, as a dependency, to Commons Collections. (As everyone agrees.)
If you were wondering about Ordering
, note that it implements Comparator
. It's pretty handy especially for more complex sorting needs, allowing you for example to chain several Orderings using compound()
. Read Ordering Explained for more!
I would implement a null safe comparator. There may be an implementation out there, but this is so straightforward to implement that I've always rolled my own.
Note: Your comparator above, if both names are null, won't even compare the value fields. I don't think this is what you want.
I would implement this with something like the following:
// primarily by name, secondarily by value; null-safe; case-insensitive
public int compareTo(final Metadata other) {
if (other == null) {
throw new NullPointerException();
}
int result = nullSafeStringComparator(this.name, other.name);
if (result != 0) {
return result;
}
return nullSafeStringComparator(this.value, other.value);
}
public static int nullSafeStringComparator(final String one, final String two) {
if (one == null ^ two == null) {
return (one == null) ? -1 : 1;
}
if (one == null && two == null) {
return 0;
}
return one.compareToIgnoreCase(two);
}
EDIT: Fixed typos in code sample. That's what I get for not testing it first!
EDIT: Promoted nullSafeStringComparator to static.