CODEWITHSUNDEEP
javacomparatorcomparable
Remo
Remo

Reputation: 1412

Java8 Null-safe comparison

Having issues with comparing two products. I wish to compare the vintage (which is optional) attribute of each of them. But whenever this attribute is null, a NPE is thrown. I thought with Comparator.nullsLast(..) I can deal with null values... But it seems I either have a misunderstanding of how this works or there's something wrong with the code. What do I need to change to get this work null-friendly? 

@Override
public int compare(IProduct product1, IProduct product2) throws ProductComparisonException {

    Comparator<IShopProduct> comparator =
        Comparator.nullsLast(Comparator.comparing(IShopProduct::getVintage));

    return comparator.compare((IShopProduct)product1.getProvidedProductData(),
                              (IShopProduct)product2.getProvidedProductData());
}

Thanks in advance

Upvotes: 8

Views: 6523

Answers (2)

Mạnh Quyết Nguyễn
Mạnh Quyết Nguyễn

Reputation: 18235

It should be

Comparator<IShopProduct> comparator = 
            Comparator.comparing( IShopProduct::getVintage, 
                             Comparator.nullsLast(Comparator.naturalOrder()));

Comparator.nullsFirst()/nullsLast() consider null value being greater/smaller than nonNull object

Edit

This is implementation of Comparator.comparing():

public static <T, U extends Comparable<? super U>> Comparator<T> comparing(
        Function<? super T, ? extends U> keyExtractor)
{
    Objects.requireNonNull(keyExtractor);
    return (Comparator<T> & Serializable)
        (c1, c2) -> keyExtractor.apply(c1).compareTo(keyExtractor.apply(c2));
}

As you can see it calls keyExtractor.apply(c1).compareTo() so it will throw NPE if keyExtractor.apply(c1) is null

My suggested code using the following function:

public static <T, U> Comparator<T> comparing(
        Function<? super T, ? extends U> keyExtractor,
        Comparator<? super U> keyComparator)
{
    Objects.requireNonNull(keyExtractor);
    Objects.requireNonNull(keyComparator);
    return (Comparator<T> & Serializable)
        (c1, c2) -> keyComparator.compare(keyExtractor.apply(c1),
                                          keyExtractor.apply(c2));
}

Basically it will extracts the value, then pass the comparing values to Comparator.

The values will be passed to the naturalOrder() comparator which resolved to value1.compareTo(value2). Normally it will throw NPE but we have wrapped it with Comparator.nullsLast which have special null handler.

Upvotes: 8

Ousmane D.
Ousmane D.

Reputation: 56423

This overload of the comparing method will throw an exception if the passed in key extractor function is null or the property extracted is null. Since, you mentioned the vintage property can be null at times then this is the cause of the NullPointerException.

An alternative, to overcome the problem is to use this comparator:

 Comparator<IShopProduct> comparator = 
      Comparator.comparing(IShopProduct::getVintage,
                Comparator.nullsLast(naturalOrder()));

The key extractor i.e IShopProduct::getVintage is the function used to extract the sort key.

The key comparator i.e. the Comparator.nullsLast(Comparator.naturalOrder()) is used to compare the sort key.

Comparator.naturalOrder() here simply returns a comparator that compares Comparable objects in natural order.

Upvotes: 5

Related Questions