-2

I have googled and checked thoroughly all the questions and answers of "Comparison method violates its general contract" error. I am not able to figure out in scenario what is going wrong. I have written a comparator class which sometime throws this exception in CI. I am only using Integer obj.compareTo method in my NumberStringComparator and this class is used in Arrays.sort method.

Arrays.sort(valuesArray, new NumberStringComparator());

Class code

private static class NumberStringComparator implements Comparator<Object>
{
    @Override
    public int compare (Object entry1, Object entry2) {

        if(!(entry1 instanceof String && entry2 instanceof String)){
            throw new ClassCastException();
        }

        try{
            Integer a1 = Integer.valueOf((String)entry1);
            Integer a2 = Integer.valueOf((String)entry2);
            return a1.compareTo(a2);
        }
        catch(Exception e){
            try{
                return ((String)entry1).compareTo((String)entry2);
            }
            catch(Exception e1){
                throw new ClassCastException();
            }
        }
    }
}

edited: sample values (more than thousand values) used are :

1234567894, 1234567893, 1234567892, 1234567891, 1234567890, 12345678949, 491940032, 491940033, 12345678947, 491940030, 12345678948

Navin Singh
  • 1
  • 3
  • can you post the valuesArray too? – ΦXocę 웃 Пepeúpa ツ Mar 01 '17 at 10:33
  • 1
    Why do you throw `ClassCastException` instead of e.g. `IllegalArgumentException`? If you wanted to throw `ClassCastException`, just don't catch the exception in the first place. Also, why don't you extend `Comparator` if you only allow strings to be compared? – tobias_k Mar 01 '17 at 10:34
  • Consider the case when you have both a `String` (not representable as an `Integer`, like `"abc"`) and an `Integer`. – Tunaki Mar 01 '17 at 10:41
  • 1
    @Tunaki Is this really a duplicate? The way I understand the question, OP is not asking "What does that exception mean?" but "Why is my comparison violating that rule?". – tobias_k Mar 01 '17 at 10:43
  • @tobias_k From reading the question, I don't see OP knowing the transitivity rule and checking that it verifies it, hence the duplicate mark (because it doesn't). – Tunaki Mar 01 '17 at 10:46
  • @Tunaki Hey tunaki i understand the rule of transitivity and read/understand other scenario too. I am asking here for the main root cause of that exception since it is Integer entry1.compareTo method used. – Navin Singh Mar 01 '17 at 10:52
  • @tobias_k Yeah i agree with your first comment about exception and generic but this is the legacy code hence haven't touch that part. – Navin Singh Mar 01 '17 at 11:01
  • @Tunaki I have tested the string case with "abc" and Integer both it works file and i have mentioned it that this exception appears sometime only. Values are passing through GUI so null values are also not present. – Navin Singh Mar 01 '17 at 11:04
  • So, why do you think your comparator is transitive? Reason through comparing a String with 2 different Integers. – Tunaki Mar 01 '17 at 11:06
  • I couldn't reproduce the exception, but an example where the comparator behaves weird is if you call it with the lists {"0x", "02", "1"} and {"0x", "1", "02"}. Since they have the same elements, they should produce the same results, but the first is sorted to {"1", "02", "0x"} and the second to {"0x", "1", "02"}. With the comparator above "0x" < "1" < "02" but "0x" > "02" – Harald Gliebe Mar 01 '17 at 11:07
  • @Tunaki In our actual environment String ("abc") is never used. Always integer values are being passed. I had tested with some string and alphnumeric values too in my IDE and it works fine. So now if we consider only Integer values Integer obj.compareTo method is used so i fail to understand whether there could be some value which would make it to throw exception or what. – Navin Singh Mar 01 '17 at 11:15
  • 1
    Some of these values are too big for the Integer type, so the valueOf method will throw a NumberFormatException and they will be compared as strings. For example: "11" < "12345678948" < "2" and "2" < "11" is true with your comparator, but contradicts the transitivity requirement. – Harald Gliebe Mar 01 '17 at 11:28
  • @Harald True, but we have been using these big values and exception is thrown only sometime. I will do the thorough testing and if found would use Long instead of Integer. thanks a lot – Navin Singh Mar 01 '17 at 11:57

1 Answers1

2

The problem with this solution is that your comparator is not transitive.

Very likely you have in your array that taken three elements A, B and C they not have a transitive relation.

In other words, when a comparator is not transitive you can have that A > B and B > C but also C > A.

freedev
  • 25,946
  • 8
  • 108
  • 125