NullPointerException when using Comparator with nullsLast

Question

I know there are many similar questions about the same issue. However, I've tried the solutions provided and none worked for me. I am 100% sure it is something basic I am missing, but I am not able to find what is wrong, hence asking it here...

I have a custom object like this:

public class ClassA {
    private int id;
    private Integer min1;
    private Integer min2;
}

In another class (ClassB) I have a list of ClassA objects that I need to sort. First items should be those who have a lower value in any of the min properties. In case two objects have the same value, then does not matter which one goes first. For example, if I have these objects:

ID	min1	min1
1	10	null
2	null	30
3	80	null
4	23	null
5	10	null

Order should be (ID): 1, 5, 4, 2, 3.

To achieve this I have a method that returns a Comparator:

public static Comparator<ClassA> compareMins(Function<ClassA, Integer> min) {
    return Comparator.comparing(minuto, Comparator.nullsLast(Comparator.naturalOrder()));
}

In ClassB I get the Comparator and sort the list as follows:

Comparator<ClassA> comparator = ClassA.compareMins(ClassA::getMin1)
    .thenComparing(ClassA::getMin2);
lista.sort(comparator);

However, it is throwing a NullPointerException when comparing objects ID 1 and 5. I guess that as min1 has the same value, it tries to compare min2, which are null, and hence, throwing this exception. However, I am unable to find a solution to this. Any help would be really appreciated.

Answer 1

It looks like you've forgotten to use your nulls comparator compareMins a second time:

Comparator<ClassA> comparator = compareMins(ClassA::getMin1)
                 .thenComparing(compareMins(ClassA::getMin2));

As you have it now, using .thenComparing(ClassA::getMin2), would evaluate the two instances of min2 and would call a.min2.compareTo(b.min2) rather than the nulls friendly check you have created in compareMins. Hence NullPointerException.

Also, you have mixed min and minuto in the declaration of compareMins, this could be source of NPE => replace minuto with min.

public static Comparator<ClassA> compareMins(Function<ClassA, Integer> min) {
    return Comparator.comparing(min, Comparator.nullsLast(Comparator.naturalOrder()));

}

Answer 2

Programming isn't magic and it isn't like humans. "Do what I mean" is not how it works.

Comparator<ClassA> comparator = ClassA.compareMins(ClassA::getMin1)
   .thenComparing(ClassA::getMin2);
lista.sort(comparator);

This dutifully does exactly what you wrote, which is not what you meant. The nullsLast logic is applied by what compareMins does. You then chain thenComparing(ClassA::getMin2) which, rather obviously, does not do any nulls last. How could it? The compiler can't sniff out that 'hey, compareMins appears to do some nullsLast magic, do I think the programmer intended for the same nullsLast logic to apply to getMin2? Eh, I'll take a wild stab in the dark: Probably yeah so I'll just do that'.

Not, obviously, how compilers work.

By default all comparators NPE on nulls because nulls do not have any obvious ordering, and optimally don't even have an ordering between themselves (null is neither above nor below nor equal to any other null - null is best treated as 'unknown'. Given 2 unknown values it is not possible to say which one is higher or lower, nor is it possible to decree that they are equal).

You can fix it in the style you've been going so far but the style you've chosen doesn't sound right.

A much more obvious way to go is to make a method or function that does the job of 'return the lower of these 2 values, ignoring nulls'. Which isn't something the standard library has; it's a programming language where you program what you want. It's not a box of lego bricks where the job consists solely of finding the right bricks and snapping them together. Sometimes you gotta make your own, so to speak.

class classA {
 ....

 public int getLowest() {
   return
     min1 == null ? min2 :
     min2 == null ? min1 :
     Math.min(min1, min2);
 }
}

listA.sort(Comparator.comparingInt(ClassA::getLowest));

This fails only if both min1 and min2 are null for any instance of ClassA. Crashing is probably correct (it seems arbitrary / an error of attempting to blast right past the problem that something isn't known), but if you don't want that, you'd go with:

class classA {
 ....

 public Integer getLowest() {
   return
     min1 == null ? min2 :
     min2 == null ? min1 :
     Math.min(min1, min2);
 }
}

listA.sort(
  Comparator.comparing(
    ClassA::getLowest,
    Comparator.nullsLast(Comparator.naturalOrder())
  )
);