I have a class
LoanAccount
that contains an attributescreationDate
andloanAmount
.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
-
2\$\begingroup\$ @dfhwze I've posted the real class. \$\endgroup\$Cristian Iacob– Cristian Iacob2019年09月23日 09:02:08 +00:00Commented Sep 23, 2019 at 9:02
-
3\$\begingroup\$ some useful comparison strategies in Java: baeldung.com/java-8-comparator-comparing and stackoverflow.com/a/20093589/11523568 \$\endgroup\$dfhwze– dfhwze2019年09月23日 09:29:04 +00:00Commented Sep 23, 2019 at 9:29
2 Answers 2
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.
-
2\$\begingroup\$ Two more things... First you could pass the inner comparator itself to
Comparator.nullsFirst()
, which would make that comparator handlenull
automatically (thus avoiding the three if at the beginning). Second, that whole thing become your comparator, so you don't need the LoanAccountAmountComparator class at all. Simply move the comparator construction to some other place (for example, the LoadAccount class), and store it into a public static field (lets sayACCOUNT_BY_AMOUNT_AND_DATE_COMPARATOR
)... \$\endgroup\$James– James2019年09月23日 18:09:32 +00:00Commented Sep 23, 2019 at 18:09 -
\$\begingroup\$ @jwatkins You are right. \$\endgroup\$dariosicily– dariosicily2019年09月23日 20:27:11 +00:00Commented Sep 23, 2019 at 20:27
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()
andComparator.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 noif
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 forcreationDate
. 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()
.