Best way to implement compare method of Comparator in java?

Question

I have written a comparator which sorts by ascending order as below. which is working well.

Collections.sort(resultList,new Comparator<MyModelClass>() {
            @Override
            public int compare(MyModelClass o1, MyModelClass o2) {
                Integer id1= o1.getId();
                Integer id2= o2.getId();
                if(id1 == null && id2 == null) {
                    return 0;               
                }else if(id1 != null && id2 == null) {
                    return -1;
                } else if (id1 == null && id2 != null) {
                    return 1;
                } else {                
                    return id1.compareTo(id2);
                }
            }
        });

is it good to implement like this? Please help me?

Thanks!

Solution 1

It looks good for readability, but a slightly more efficient way might be:

public int compare(MyModelClass o1, MyModelClass o2) {
    Integer id1= o1.getId();
    Integer id2= o2.getId();
    if (id1 == null) {
        return id2 == null ? 0 : 1;
    }
    if (id2 == null) {
        return -1;
    }
    return id1.compareTo(id2);
}

or even:

public int compare(MyModelClass o1, MyModelClass o2) {
    Integer id1= o1.getId();
    Integer id2= o2.getId();
    if (id1 == null) {
        return id2 == null ? 0 : 1;
    }

    return id2 == null ? -1 : id1.compareTo(id2);
}

Solution 2

If you need the null-safe comparison logic in several comparators then I would suggest using a static helper in a utility class like this:

public static int compare(Comparable c1, Comparable c2) {
    return c1 == null
               ? (c2 == null ? 0 : 1)
               : (c2 == null ? -1 : c1.compareTo(c2));
}

The Comparator could then be simplified to:

public int compare(MyModelClass o1, MyModelClass o2) {
    return CompareHelper.compare(o1.getId(), o2.getId());
}

Solution 3

No, it is not a good implementation.

The java.util.List specification says you can have nulls in a list and in some cases you can have multiple nulls. Your Comparator will fail with a NullPointerException as soon as it tries to do o?.getId() on a null element.

What I generally do is make my class implement java.lang.Comparable, then I can use a Map to sort the elements as I add them. One generally has to build the list so why not build a TreeMap instead?

If you are reusing your class and want to sort it in different ways you can create a TreeMap with a Comparator in the constructor so that there is no need to explicitly sort it.

Solution 4

If getId() returns an int, you can simply go with return id1.compareTo(id2), this will give you the right result. Hope this helps.

Solution 5

Yes it is, I sort of do the same thing. One remark would be that you can use so nullsafe comparison tools such as Apache Commons Collections's NullComparator to ditch all those null checkings in your code:

http://commons.apache.org/collections/api-2.1.1/org/apache/commons/collections/comparators/NullComparator.html#compare(java.lang.Object, java.lang.Object)

View more solutions

Comments

[user1016403]

Thanks for your reply. for descending order simply we need to reverse them right? Please reply.

[Per]

Yes, simply switch id1 for id2.

[Per]

As David Newcomb points out, there's the risk of null values in Lists, which had escaped my mind. Consider his advice as well.

[viktor]

If you need to implement the second iterator for sorting in opposite order use Collections.reverseOrder(Comparator<T> cmp) instead.

[user1016403]

Thanks for youre reply. what is the logic behind sorting as i did in compare() method. Please consider i have 5 Employee objects and their ids are 20,10.40,20,50. now how does it sorts based on returning -1 or 0 or +1? please help me.

[Per]

@user1016403 Sorting algorithms rely on comparing two values. For each compare, -1 means that o1 should come before o2 in the sorted result, 0 means don't care, 1 means after. The sorting algorithm invokes compare() a sufficient number of times for different objects to make sure the list is completely sorted.