Search code examples
javacomparisoncomparecomparatorcompareto

Comparison method violates its general contract and method compareTo


I have a class Contact with fields firstName, lastName and emails. I need to sort them using Collection.sort(...), but I got an exception:

java.lang.IllegalArgumentException: Comparison method violates its general contract!

My compareTo method:

    @Override
public int compareTo(Contact another) {
    int compareFirstName = 0;
    if (this.getFirstName() != null && another.getFirstName() != null) {
        compareFirstName = this.getFirstName().compareToIgnoreCase(
                another.getFirstName());

        if (compareFirstName == 0) {
            int compareLastName = 0;
            if (this.getLastName() != null && another.getLastName() != null) {
                compareLastName = this.getLastName().compareToIgnoreCase(
                        another.getLastName());

                if (compareLastName == 0) {
                    int compareEmail = 0;
                    if (this.getEmails() != null
                            && another.getEmails() != null) {
                        compareEmail = this.getEmails()
                                .compareToIgnoreCase(another.getEmails());

                        return compareEmail;
                    } else {

                        return 0;
                    }
                } else {
                    return compareLastName;
                }
            } else {
                int compareEmail = 0;
                if (this.getEmails() != null && another.getEmails() != null) {
                    compareEmail = this.getEmails().compareToIgnoreCase(
                            another.getEmails());

                    return compareEmail;
                } else {

                    return 0;
                }
            }
        } else {

            return compareFirstName;
        }
    } else {
        int compareLastName = 0;
        if (this.getLastName() != null && another.getLastName() != null) {
            compareLastName = this.getLastName().compareToIgnoreCase(
                    another.getLastName());

            if (compareLastName == 0) {
                int compareEmail = 0;
                if (this.getEmails() != null && another.getEmails() != null) {
                    compareEmail = this.getEmails().compareToIgnoreCase(
                            another.getEmails());

                    return compareEmail;
                } else {

                    return 0;
                }
            } else {

                return compareLastName;
            }
        } else {
            int compareEmail = 0;
            if (this.getEmails() != null && another.getEmails() != null) {
                compareEmail = this.getEmails().compareToIgnoreCase(
                        another.getEmails());

                return compareEmail;
            } else {

                return 0;
            }
        }
    }
}

Please help me to find error in my compareTo method. Thanks.


Solution

  • Your implementation does violate the contract.

    Suppose you have 3 Contacts :

    contact1 : First Name = "John", Last Name = "Doe", Email = "[email protected]"
    contact2 : First Name = "John", Last Name = "Doe", Email = null
    contact3 : First Name = "John", Last Name = null, Email = "[email protected]"
    

    Based on your logic :

    contact1.compareTo(contact2) returns 0 (since they have the same first and last name).
    contact2.compareTo(contact3) also returns 0 (since you only compare by first name).
    But contact1.compareTo(contact3) doesn't return 0 (since they have different emails).

    compareTo must be transitive.

    The way to fix this is not to ignore a property that is null only in one of the contacts you are comparing. For example, if this.getLastName()==null && another.getLastName() != null, return 1 (assuming you want to order the null last names after the non-null last names).