handle null in Comparator class

Question

I have a class LoanAccount that contains an attributes creationDate and loanAmount.

I want to implement a Comparator that sorts the accounts by loan amount in descending order. When the loan amounts are the same, sorts the accounts ascending by date.

The code look like this:

Class

public class LoanAccount {
    private String id;
    private Date creationDate;
    private BigDecimal loanAmount;
}

Comparator

public class LoanAccountAmountComparator implements Comparator<LoanAccount> {

    @Override
    public int compare(LoanAccount o1, LoanAccount o2) {
        if (o1 == null) {
            if (o2 == null) {
                return 0;
            }
            return -1;
        }
        if (o2 == null) {
            return 1;
        }
        if (o1.getLoanAmount() == null) {
            if (o2.getLoanAmount() == null) {
                return 0;
            }
            return -1;
        }
        if (o2.getLoanAmount() == null) {
            return 1;
        }

        int comparedLoanAmount = o2.getLoanAmount().compareTo(o1.getLoanAmount());
        if (comparedLoanAmount == 0) {
            if (o1.getCreationDate() == null) {
                if (o2.getCreationDate() == null) {
                    return 0;
                }
                return -1;
            }
            if (o2.getCreationDate() == null) {
                return 1;
            }
            return o1.getCreationDate().compareTo(o2.getCreationDate());
        } else {
            return comparedLoanAmount;
        }
    }
}

Objects o1, o2 can be null. Fields creationDate and loanAmount can be null.

I want to ask you if there are better ways to handle null pointer exception in compareTo method.

For me it looks like there are too many if conditions

Answer 1

it is possible to handle null pointer exception using Comparator.comparing static method. If you want to comparing loanAmount fields in your objects and you want that every not null value is greater than null you can use Comparator.nullFirst method in combination with Comparator.comparing like the below code:

Comparator.comparing(LoanAccount::getLoanAmount,
                     Comparator.nullsFirst(Comparator.naturalOrder()))

NaturalOrder is a method returning a comparator that compares Comparable objects in natural order. You can also combine more comparators in a chain with method Comparator.thenComparing like the code below:

Comparator
        .comparing(LoanAccount::getLoanAmount,
                   Comparator.nullsFirst(Comparator.naturalOrder()))
        .thenComparing(LoanAccount::getCreationDate, 
                       Comparator.nullsFirst(Comparator.naturalOrder()))
        .compare(o1, o2);

Now you can rewrite your comparator with equal behaviour shortly:

public class LoanAccountAmountComparator
        implements Comparator<LoanAccount> {

    @Override
    public int compare(LoanAccount o1, LoanAccount o2) {
        if (o1 == null && o2 == null) return 0;
        if (o1 == null) return -1;
        if (o2 == null) return 1;
        return Comparator
                .comparing(LoanAccount::getLoanAmount,
                           nullsFirst(naturalOrder()))
                .thenComparing(LoanAccount::getCreationDate,
                               nullsFirst(naturalOrder()))
                .compare(o1, o2);
    }
}

Note: the use of class Date is discouraged, it is better if you use instead the java.time package classes for time related code.

Answer 2

Here is another twist, roughly similar to dariosicily's proposal.

It differs in the following points:

• It create the aggregated comparator only once, then store it in a static field. Why? Because functions such as Comparator.nullsFirst() and Comparator.comparing() implies object allocation, which you definitely want to avoid if your comparator is called from tight loops (for example, a sort or tree insertion algorithm).
• Null checks on the LoadAccount objects themselves have also been delegated to Comparator.nullsFirst(). That means that absolutely no if statement is required!
• I moved that comparator to a public, static field of a utility class. I have learned from personal experience that comparators on domain model objects very often come in "families". In different places, you want different sorting strategies, for the same objects. Here, for demonstration, I also included one comparator for loadAmount (that is, without fallback on tie), and another one for creationDate. But you can see how this idea can be generalized.
• In this sample, I have adopted an uncommon indentation strategy. I think this is justified in this case because it helps make it easier to see which fields are sorted by each comparator, and in which order. This is of great importance when you have comparators that involve a significant number of fields.

    public class LoanAccountAmountComparators {

        /**
         * A comparator that sort LoanAccounts, by load's amount, in decreasing order.
         */
        public static final Comparator<LoanAccount> BY_AMOUNT = 
            Comparator.nullsFirst(
                Comparator.comparing(
                    LoanAccount::getLoanAmount, Comparator.nullsFirst(Comparator.reverseOrder())
                );
            );

        /**
         * A comparator that sort LoanAccounts, by creation date, in ascending order.
         */
        public static final Comparator<LoanAccount> BY_DATE = 
            Comparator.nullsFirst(
                Comparator.comparing(
                    LoanAccount::getCreationDate, Comparator.nullsFirst(Comparator.naturalOrder())
                );
            );

        /**
         * A comparator that sort LoanAccounts, by creation amount (in descending order),
         * then by date (in ascending order).
         */
        public static final Comparator<LoanAccount> BY_AMOUNT_AND_DATE = 
            Comparator.nullsFirst(
                Comparator.comparing(
                    LoanAccount::getLoanAmount, Comparator.nullsFirst(Comparator.reverseOrder())
                ).thenComparing(
                    LoanAccount::getCreationDate, Comparator.nullsFirst(Comparator.naturalOrder())
                );
            );
    }

It should be noted that, in this example, all fields involved are indeed some kind of objects. If, however, your comparator would involve fields containing primitive types, then you should use the corresponding Comparator.comparing<primitiveType> function (that is, never let some primitive be boxed to object only so it can be compared to Comparator.naturalOrder().