Search code examples
javajava-streamcomparator

Unclear why comparator in stream violates general contract


I'm getting the following error when sorting a collection:

Caused by: java.lang.IllegalArgumentException: Comparison method violates its general contract!
    at java.util.TimSort.mergeLo(TimSort.java:777)
    at java.util.TimSort.mergeAt(TimSort.java:514)
    at java.util.TimSort.mergeCollapse(TimSort.java:441)
    at java.util.TimSort.sort(TimSort.java:245)

This is the comparator and the stream in which it is used.:

Comparator<Task> comparator = comparing(Task::getAssigneeSite, nullsLast(naturalOrder())).thenComparing(Task::getDueDate,
        nullsLast(naturalOrder()));

Collection<Task> groupTasks = findTasks(site, dossierAdministratorGroup).stream()
        .sorted(comparator)
        .collect(toList());

I've found the following answer for a similar problem:

Exception 'Comparison Method Violates Its General Contract!' when comparing two date properties of an object

But I don't see how this solves the problem.

Wrapping the comparator in null is only useful when the bean itself can be null, which is not the case (the tasks are never null).

To my understanding the nullsLast(...) inside the comparator for the assigneeSite means that nulls should be considered greater than assigneeSite objects, same goes for the DueDate. So I'm stumped as to what the contract violation could be.

Can anyone explain the contract violation that occurs inside this comparator?


Solution

  • So, this was probably very stupid on my part, but the problem was in an already existing comparator in the 'Site' class that I didn't verify.

        @Override
        public int compareTo(Site other) {
            if (this.code < other.getCode()) {
                return -1;
            } else {
                return 1;
            }
        }
    

    There's an obvious issue here as the comparator does not specify the case where both code fields are equal. Besides that it also, does not take into account either Integer object possibly being null.

    A correct, nullsafe implementation (using java 8's Comparator.comparing method) would be:

            return comparing(Site::getCode, nullsLast(naturalOrder())).compare(this, other);
    

    I'm putting this answer here to remind others to check all the comparators when getting a contract violation error.