HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Comparing SKU numbers

Submitted by: @import:stackexchange-codereview··
0
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 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.