CODEWITHSUNDEEP
javacollectionsmergesorttimsort
harish km
harish km

Reputation: 33

Java Error: java.lang.IllegalArgumentException: Comparison method violates its general contract

I am working on a old application which was originally written in Java 6 and which was upgraded to Java 7 a couple of years ago.

In this application i am using a Collection.Sort to sort a list with a custom compare method by implementing Comparator interface. Type of objects in the list are CompanySchedule which have 3 properties companyName, Schedule and expirationdate.

List can contain multiple objects with same companyName but with unique expiration date. Below compare function sorts the list in ascending order of companyName and with in the same companyName list descending order of expiration date. Below is the method implementation. 

    public int compare(CompanySchedule c1, CompanySchedule c2) {
        int returnVal = 0;
        int value = c1.getCompany().getName().compareTo(c2.getCompany().getName());
        if (value == 0){
            if (c1.getUseExpirationDate() == null || c2.getUseExpirationDate() == null){
                returnVal = -1;
            }
            else{
                int chkdate = c1.getUseExpirationDate().compareTo(c2.getUseExpirationDate());
                if (chkdate == 0){
                    returnVal = 0;
                }
                else if (chkdate > 0){
                    returnVal = -1;
                }
                else if (chkdate < 0){
                    returnVal = 1;
                }
            }
        }
        else if (value < 0){
            returnVal = -1;
        }
        else if (value > 0){
            returnVal = 1;
        }


        return returnVal;
    }

I know that when the transitive property is not met in the compare method implementation above error java.lang.IllegalArgumentException: Comparison method violates its general contract will be thrown.

can some one help in identifying the where this method will violates the transitive property. Thanks for the Help.

Upvotes: 1

Views: 419

Answers (2)

harish km
harish km

Reputation: 33

I know I am late to this, once I found the solution I had basically forgotten I had asked this question. so as I mentioned the application was written in Java 6 and then was upgraded to Java 7, as per my understanding the internal implementation of Collections. Sort method changes the algorithm used from Merge sort to Tim Sort, Merge Sort ignore the Transitivity property of the compare method implementation, however Tim sort required the the compare method implementation to be transitive else the above exception would be thrown, since we had an legacy app we did not want to change anything from the code so we used jvm argument java.util.Arrays.useLegacyMergeSort=true Merge Sort internally in the sort implementation

Upvotes: 1

galusben
galusben

Reputation: 6372

I think that one problem is here:

if (c1.getUseExpirationDate() == null || c2.getUseExpirationDate() == null){
                returnVal = -1;
}

If a.getUseExpirationDate() == null and also b.getUseExpirationDate() == null, you will get that a < b and b < a which means a < a.

This breaks consistency. There might be more problems in this method but I have not checked it all.

Good luck.

EDIT

How about this code?

public int compare(CompanySchedule c1, CompanySchedule c2) {
        int returnVal = 0;
        int value = c1.getCompany().getName().compareTo(c2.getCompany().getName());
        if (value == 0) {
            if (c1.getUseExpirationDate() == null && c2.getUseExpirationDate() != null) {
                returnVal = -1;
            } else if (c1.getUseExpirationDate() != null && c2.getUseExpirationDate() == null) {
                returnVal = 1;
            } else if (c1.getUseExpirationDate() == null && c2.getUseExpirationDate() == null) {
                returnVal = 0;
            } else {
                int chkdate = c1.getUseExpirationDate().compareTo(c2.getUseExpirationDate());
                if (chkdate == 0) {
                    returnVal = 0;
                } else if (chkdate > 0) {
                    returnVal = -1;
                } else if (chkdate < 0) {
                    returnVal = 1;
                }
            }
        } else if (value < 0) {
            returnVal = -1;
        } else if (value > 0) {
            returnVal = 1;
        }


        return returnVal;
    }

I have tried not to change it too much, for comparability, but it should be refactored. Basically it determines that nulls are smaller than other values.

Upvotes: 1

Related Questions