patternjavaModerate
Comparing SKU numbers
Viewed 0 times
skucomparingnumbers
Problem
Sonar complains that I have too many
return statements. How can I reduce the return statement count nicely?public int compare(Sku o1, Sku o2) {
if (o1 == null) {
return (o2 == null) ? 0 : 1;
} else if (o2 == null) {
return -1;
}
if (o1.getSkuNumber() == null) {
return o2.getSkuNumber() == null ? 0 : 1;
} else if (o2.getSkuNumber() == null) {
return -1;
}
return o1.getSkuNumber().compareTo(o2.getSkuNumber());
}Solution
Since you state that you're using Java 8, you should use the variety of new
-
When
-
We then need to compare by
edit: @Simon mentioned a good point that since your existing
Comparator static methods to extract the relevant fields for comparison:public int compare(Sku o1, Sku o2) {
Comparator c1 = Comparator.nullsLast(null);
return c1.thenComparing(Sku::getSkuNumber,
Comparator.nullsLast(Comparator.naturalOrder())).compare(o1, o2);
}-
When
Comparator.nullsLast(Comparator) is given a null argument, it considers two non-null argument to be equal. This is the basis of our first comparator.-
We then need to compare by
getSkuNumber(), so we pass the method reference Sku::getSkuNumber to Comparator.thenComparing(Function, Comparator) as our second comparator. However, since SKU numbers may be null, we use a nested (or tertiary?) null-friendly comparator, Comparator.nullsLast(Comparator) again, as the second argument. The final comparator in use is just Comparator.naturalOrder(), which performs the final straightforward integer comparison if both SKU numbers are non-null.edit: @Simon mentioned a good point that since your existing
compare(Sku, Sku) method is likely to be the implementation for implementing Comparable, you should consider making the Comparator I suggested above a public static final field, so that it doesn't have to be created every time, e.g.:public static final Comparator COMPARATOR =
Comparator.nullsLast((Comparator) null)
.thenComparing(Sku::getSkuNumber,
Comparator.nullsLast(Comparator.naturalOrder()));Code Snippets
public int compare(Sku o1, Sku o2) {
Comparator<Sku> c1 = Comparator.nullsLast(null);
return c1.thenComparing(Sku::getSkuNumber,
Comparator.nullsLast(Comparator.naturalOrder())).compare(o1, o2);
}public static final Comparator<Sku> COMPARATOR =
Comparator.nullsLast((Comparator<Sku>) null)
.thenComparing(Sku::getSkuNumber,
Comparator.nullsLast(Comparator.naturalOrder()));Context
StackExchange Code Review Q#104313, answer score: 12
Revisions (0)
No revisions yet.