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.
Advertisement
Answer
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.