compareTo comparison method violates its general contract

Question

I want to compare two "Recipients" by the dateLastContact, and if it's the same, by the address. So this is my code:

public class RecipientComparator implements Comparator<Recipient> {
    @Override
    public int compare(Recipient o1, Recipient o2) {
        if (o1.isDateLastContactNull() || o2.isDateLastContactNull()) {
            if (o1.isDateLastContactNull() && o2.isDateLastContactNull()) {
                return o1.getAddress().compareTo(o2.getAddress());
            } else if (o1.isDateLastContactNull()) {
                return -1;
            } else if (o2.isDateLastContactNull()) {
                return -1;
            }
        } else {
            if (o1.getDateLastContact().equals(o2.getDateLastContact())) {
                return o1.getAddress().compareTo(o2.getAddress());
            } else
                return o1.getDateLastContact().compareTo(o2.getDateLastContact());
        }
        return 0;
    }
}

And I always have this error :

Exception in thread "main" java.lang.IllegalArgumentException: Comparison method violates its general contract!
    at java.util.TimSort.mergeHi(TimSort.java:899)
    at java.util.TimSort.mergeAt(TimSort.java:516)
    at java.util.TimSort.mergeCollapse(TimSort.java:439)
    at java.util.TimSort.sort(TimSort.java:245)
    at java.util.Arrays.sort(Arrays.java:1512)
    at java.util.ArrayList.sort(ArrayList.java:1462)
    at managers.RecipientManager.getAllRecipientFromAPI(RecipientManager.java:28)
    at com.company.Main.main(Main.java:18)

I have tried a lot of things but now, I don't know what to do. Can you help me?

The recipient class:

package recipientPackage;

import paramPackage.Param;

import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Date;

public class Recipient {
    private String recipientID;
    private String address;
    private String status;
    private int contactCount;
    private Date dateLastContact;
    Param param;
    private String[] attributes = {"recipientID", "address", "status", "contactCount", "dateLastContact"};

    Recipient(String[] recipientBrut, boolean isComeFromMailing) {
        if (isComeFromMailing) {
            createRecipientMailing(recipientBrut);
        } else {
            createRecipientSurvey(recipientBrut);
        }
    }

    public void createRecipientMailing(String[] recipientBrut) {
        this.setRecipientID(recipientBrut[0].substring(recipientBrut[0].indexOf(':') + 1).replaceAll("\"", ""));
        this.setAddress(recipientBrut[1].substring(recipientBrut[1].indexOf(':') + 1).replaceAll("\"", ""));
        this.setStatus(recipientBrut[3].substring(recipientBrut[3].indexOf(':') + 1).replaceAll("\"", ""));

        try {
            this.setDateLastContactForMailing(recipientBrut[5].substring(recipientBrut[5].indexOf(':') + 1).replaceAll("\"", ""));
            this.setContactCount(Integer.parseInt(recipientBrut[4].substring(recipientBrut[4].indexOf(':') + 1).replaceAll("\"", "")));
        } catch (IndexOutOfBoundsException e) {
            this.setDateLastContactForMailing(null);
        }catch (NumberFormatException e){
            e.printStackTrace();
        }
    }

    public void createRecipientSurvey(String[] recipientBrut) {
        setAddress(recipientBrut[0]);
        setStatus(recipientBrut[1]);
        setDateLastContactForSurvey(recipientBrut[2]);
        param.setParam_point_collecte(recipientBrut[5]);
        param.setParam_langue(recipientBrut[6]);
        param.setParam_semaine(recipientBrut[7]);
        param.setParam_periode(recipientBrut[8]);
        param.setParam_envoi(recipientBrut[9]);
        param.setParam_type_visiteur(recipientBrut[10]);
        param.setParam_saison(recipientBrut[11]);
    }

    private void setDateLastContactForMailing(String dateLastContact) {
        if (dateLastContact != null) {
            SimpleDateFormat formatter = new SimpleDateFormat("yyy-MM-dd'T'HH:mm:ss");
            try {
                this.dateLastContact = formatter.parse(dateLastContact);
            } catch (ParseException e) {
                e.printStackTrace();
            }
        } else
            this.dateLastContact = null;

    }

    private void setDateLastContactForSurvey(String dateLastContact) {
        if (!dateLastContact.equals("")) {
            SimpleDateFormat formatter = new SimpleDateFormat("dd/MM/yyyy HH:mm:ss");
            try {
                this.dateLastContact = formatter.parse(dateLastContact);
            } catch (ParseException ignored) {
            }
        } else
            this.dateLastContact = null;

    }
    public Boolean isDateLastContactNull() {
        return (dateLastContact == null);
    }

    public String getRecipientID() {
        return recipientID;
    }

    public void setRecipientID(String recipientID) {
        this.recipientID = recipientID;
    }

    public String getAddress() {
        return address;
    }

    public void setAddress(String address) {
        this.address = address;
    }

    public String getStatus() {
        return status;
    }

    public void setStatus(String status) {
        this.status = status;
    }

    public int getContactCount() {
        return contactCount;
    }

    public void setContactCount(int contactCount) {
        this.contactCount = contactCount;
    }

    public Date getDateLastContact() {
        return dateLastContact;
    }

    public void setDateLastContact(Date dateLastContact) {
        this.dateLastContact = dateLastContact;
    }

    public String[] getAttributes() {
        return attributes;
    }

    public void setAttributes(String[] attributes) {
        this.attributes = attributes;
    }
}

Answer 1

You are violating the contract of the compare method, your relationship is not transitive. "The implementor must ensure that sgn(compare(x, y)) == -sgn(compare(y, x)) for all x and y"

Consider this:

public static void main(String[] args){
    Recipient r1; // with isDateLastContactNull() == true;
    Recipient r2; // with isDateLastContactNull() == false;
    RecipientComparator rc = new RecipientComparator();

    System.out.println(rc.compare(r1, r2)); // -1
    System.out.println(rc.compare(r2, r1)); // -1
    // both print -1 which is not transitive.
}

The reason is because of this code:

if (o1.isDateLastContactNull() && o2.isDateLastContactNull()) {
    // if both null, return comparison of addresses
    return o1.getAddress().compareTo(o2.getAddress());
} else if (o1.isDateLastContactNull()) {
    // if first null, return -1
    return -1;
} else if (o2.isDateLastContactNull()) {
    // if second null, also return -1 ?
    return -1; // should probably be 1 instead
}

You should probably return 1 for the second condition.

---

The contract is as defined here:

int compare(T o1, T o2):

Compares its two arguments for order. Returns a negative integer, zero, or a positive integer as the first argument is less than, equal to, or greater than the second. In the foregoing description, the notation sgn(expression) designates the mathematical signum function, which is defined to return one of -1, 0, or 1 according to whether the value of expression is negative, zero or positive.

The implementor must ensure that sgn(compare(x, y)) == -sgn(compare(y, x)) for all x and y. (This implies that compare(x, y) must throw an exception if and only if compare(y, x) throws an exception.)

The implementor must also ensure that the relation is transitive: ((compare(x, y) > 0) && (compare(y, z) > 0)) implies compare(x, z) > 0.

Finally, the implementor must ensure that compare(x, y) == 0 implies that sgn(compare(x, z)) == sgn(compare(y, z)) for all z.

It is generally the case, but not strictly required that (compare(x, y) == 0) == (x.equals(y)). Generally speaking, any comparator that violates this condition should clearly indicate this fact. The recommended language is "Note: this comparator imposes orderings that are inconsistent with equals."

Answer 2

One obvious reason is this:

} else if (o1.isDateLastContactNull()) {
    return -1;
} else if (o2.isDateLastContactNull()) {
    return -1;
}

either o1.isDateLastContactNull() is true xor o2.isDateLastContactNull() is true
then o1 < o2.
Did you mean:

} else if (o1.isDateLastContactNull()) {
    return -1;
} else if (o2.isDateLastContactNull()) {
    return 1;
}