Why is my compareTo crashing with a general contra

Question

This question already has an answer here:

• Java error: Comparison method violates its general contract 7 answers

I'm trying to sort my custom NewsAdapter by a Date property of the Articles_Map object, and I've noticed that in cases with bigger data sets my app crashes with a java.lang.IllegalArgumentException: Comparison method violates its general contract! error.

I'm not sure if that error happens because of an int overflow, or if it is indeed related to the transitive property of the contract. And particularly for the latter, I don't know how to fix it, because as far as I understand I'm already handling the 3 possible outcomes (less than 0, 0, greater than 0).

public class NewsAdapter extends ArrayAdapter<Articles_Map> {
    Context mContext;

    public NewsAdapter(Context c, int resource) {
        super(c, resource);
        this.mContext = c;
    }

    protected void doAdd(Articles_Map another) {
        super.add(another);
    }

    public void addAll(List<Articles_Map> others) {
        for (Articles_Map a : others) {
            this.doAdd(a);
        }
        this.sort(byPublishedAtComparator);
    }

    private static final Comparator<Articles_Map> byPublishedAtComparator =
            new Comparator<Articles_Map>() {
                @Override
                public int compare(Articles_Map o1, Articles_Map o2) {
                    // needs further testing in case of nulls
                    if (o1.publishedAt == null || o2.publishedAt == null) {
                        return 0;
                    }

                    return o1.publishedAt.compareTo(o2.publishedAt);
                }
            };
}

Answer 1

Convert this variable into a class like that:

private class byPublishedAtComparator implements Comparator<Articles_Map> {
    @Override
    public int compare(Articles_Map o1, Articles_Map o2) {
        if (o1.publishedAt == null || o2.publishedAt == null) {
            return 0;
        }
        return o1.publishedAt.compareTo(o2.publishedAt);
    }
}

And instead this.sort(byPublishedAtComparator);, you have to call

Collections.sort(yourArray, new byPublishedAtComparator());

Answer 2

Your Comparator violates the transitivity requirement if publishedAt is null.

Say you have three instances of Articles_Map:

• a - with a value for publishedAt
• b - with publishedAt == null
• c - with a value for publishedtAt that is greater than as value

Now if you call your comparator with a and b or with b and c, the comparator returns 0 for both calls.

To satisfy the transitivity rule, your comparator must also return 0 if it is called with a and c. But since the field publishedAt is not null on both objects, it will return a value less than 0 if prepared like described.

To fix this, your comparator must not return 0 if only one of o1.publishedAt and o2.publishedAt is null.

For example:

private static final Comparator<Articles_Map> byPublishedAtComparator =
        new Comparator<Articles_Map>() {
            @Override
            public int compare(Articles_Map o1, Articles_Map o2) {
                // needs further testing in case of nulls
                if (o1.publishedAt == null) {
                    return (o2.publishedAt == null) ? 0 : -1;
                } else if (o2.publishedAt == null) {
                    return 1;
                }

                return o1.publishedAt.compareTo(o2.publishedAt);
            }
        };

Answer 3

Consider the following three instances:

A1 = Articles_Map{publishedAt = 1, ...)
A2 = Articles_Map{publishedAt = null, ...)
A3 = Articles_Map{publishedAt = 2, ...)

compare(A1, A2) -> 0
compare(A2, A3) -> 0
compare(A1, A3) -> -1  

That violates the contract ... and doesn't make sense. (If A1 is equal to A2 and A2 is equal to A3, then A1 should be equal to A3!)

The solution would be to treat null as being equivalent to a specific numeric value; e.g. Integer.MIN_VALUE if published_at is an int.