CODEWITHSUNDEEP
java-8
nimo23
nimo23

Reputation: 5680

Comparator.nullsFirst with null-safe-comparator

I want to have a null-safe comparator, but it does not work:

Comparator<Item> sort_high = (i1, i2)-> Double.compare(i2.getUser().getValue(), i1.getUser().getValue());

items.sort(Comparator.nullsFirst(sort_high));

However, I get a NPE, if item.getUser().getValue() (or item.getUser()) is null.

at Item.lambda$1(Item.java:270)
    at java.util.Comparators$NullComparator.compare(Comparators.java:83)
    at java.util.TimSort.countRunAndMakeAscending(TimSort.java:355)
    at java.util.TimSort.sort(TimSort.java:220)
    at java.util.Arrays.sort(Arrays.java:1438)
    at java.util.List.sort(List.java:478)

What is wrong?

Upvotes: 8

Views: 12218

Answers (3)

stanislas Pocthier
stanislas Pocthier

Reputation: 99

Another possibility, that is slightly more readable but does not distinguish between an item being null and an item having a user that is null, is the following:

Comparator<Item> sortLow = Comparator.comparing(item -> Optional.ofNullable(item)
                .map(Item::getUser)
                .map(User::getValue)
                .orElse(null),
                 Comparator.nullsFirst(Comparator.naturalOrder())                               
);

Upvotes: 0

davidxxx
davidxxx

Reputation: 131346

Comparator.nullsFirst() doesn't mean that any NullPointerException thrown during the compare() method will be caught. It says rather that a null compared object with always be less than a non null compared object and that if both are null, they are considered equal.

 <T> Comparator<T> java.util.Comparator.nullsFirst(Comparator<? super
 T> comparator)

Returns a null-friendly comparator that considers null to be less than non-null. When both are null, they are considered equal. If both are non-null, the specified Comparator is used to determine the order. If the specified comparator is null, then the returned comparator considers all non-null values to be equal.

In your case :

Comparator<Item> sort_high = (i1, i2)-> Double.compare(i2.getUser().getValue(), i1.getUser().getValue());

Compared objects are not null, that is i1 or i2 but fields of them are: i1.getUser() or i2.getUser().

You don't have a good use case for using directly Comparator.nullsFirst().
As workaround, you could use it to compare the User field in your compare() implementation as soon as one of two compared objects present a null User field.
I think that it is a good trade off between handling yourself null case and readability of the code :

private Comparator<User> comparatorUserNullFirst = Comparator.nullsFirst(Comparator.comparingDouble(User::getValue));

Comparator<Item> sortHigh = (i1, i2) -> {

    if (i1.getUser() == null || i2.getUser() == null) {
      return comparatorUserNullFirst.compare(i1.getUser(), i2.getUser());
    }

    return Double.compare(i2.getUser().getValue(), i1.getUser().getValue());
};

Upvotes: 4

Amit
Amit

Reputation: 46323

nullsFirst will take care of null Items ("i1"), but once two valid objects are found, your Comparator is invoked and you need to handle internal null references.

In your case, you could use something like:

items.sort(
  Comparator.nullsFirst(
    Comparator.comparing(Item::getUser,
      Comparator.nullsFirst(Comparator.comparingDouble(User::getValue))
    )
  )
);

(Assuming getValue() returns a double)

But I hardly recommend such convoluted code.

Upvotes: 10

Related Questions