Comparison Method violates its general contract in Java 7

Question

I am getting a "Comparison Method violates its general contract" after compiling some Java code in Java 7, and then running it.

I have read Comparison method violates its general contract! Java 7 only and realize that there is something wrong with my code that was ignored in previous versions of Java. However I cannot work out what is wrong with my code. The Collections.sort() generates the error.

My code is:

   public Comparator sortBySmoothDays() {
    Comparator c = new Comparator() {
        public int compare(Object arg0, Object arg1) {
            Date date0 = ((PosObject)arg0).getDate();
            Date date1 = ((PosObject)arg1).getDate();

            double d1 = MyUtils.calcSmoothDays(date0, new Date());
            double d2 = MyUtils.calcSmoothDays(date1, new Date());
            if (d1 >= d2) {
                return 1;
            }
            else {
                return -1;
            }   
        }
    };
    return c;
}


Comparator c = ComparatorUtils.getInstance().sortBySmoothDays();
Collections.sort(posList, c);

Can anyone help? Thank you!

JFI: Throwing this exception is a new Java7 feature. The old behavior can be configured with a new system property: java.util.Arrays.useLegacyMergeSort See http://stackoverflow.com/a/8417446/450812 — alfonx, Mar 23 '12 at 14:13

score 32 · Accepted Answer · edited May 08 '15 at 20:01

32

A comparator must return 0 if the values are equal. In your current implementation, you return 1 if they are equal. The easiest way to compare your double values correctly is to call Double.compare:

double d1 = MyUtils.calcSmoothDays(date0, new Date());
double d2 = MyUtils.calcSmoothDays(date1, new Date());

return Double.compare(d1, d2);

edited May 08 '15 at 20:01

Boann

48,794
16
117
146

answered Feb 28 '12 at 17:26

Kevin Crowell

10,082
4
35
51

It returns 1, not -1. – xehpuk May 08 '15 at 11:59
@Boann Because 3.5 years ago, I was stupid :). Feel free to edit the answer. – Kevin Crowell May 08 '15 at 16:26

NPE · Answer 2 · 2012-02-28T17:27:18.043

19

With your comparator, every object compares greater than itself: compare(x,x) always returns one.

This violates the following requirement:

The implementor must ensure that sgn(compare(x, y)) == -sgn(compare(y, x)) for all x and y.

The above requirement implies that compare(x,x) must return zero.

I would recommend reading the contract and making sure your implementation fulfils it.

In particular, if date0.equals(date1), the comparator probably ought to return zero right away, without doing any floating-point conversions and comparisons.

edited Feb 28 '12 at 17:27

answered Feb 28 '12 at 17:21

NPE

486,780
108
951
1,012

More generally, no two objects can ever be equal to each other using this method. There is no code path that yields the 0 (or equal) result. – Ian McLaird Feb 28 '12 at 17:27

score 3 · Answer 3 · answered Feb 28 '12 at 17:22

Isnt the issue that if two objects compare equal i.e. calcSmoothDays returns the same value, then you could have a situation where compare(object1,object2) == 1, and compare (object2,object1) == 1 also?

So it implies that object1 > object 2 and object2 > object 1...

Comparison Method violates its general contract in Java 7

3 Answers3

Linked

Related