0

I am using the following code that is throwing a IllegalArgumentException :

// Copyright (c) 2003-2014, Jodd Team (jodd.org). All Rights Reserved.

package jodd.util;

import java.util.Comparator;

/**
 * Compares two strings in natural, alphabetical, way.
 */
public class NaturalOrderComparator<T> implements Comparator<T> {

    protected final boolean ignoreCase;

    public NaturalOrderComparator() {
        ignoreCase = false;
    }

    public NaturalOrderComparator(boolean ignoreCase) {
        this.ignoreCase = ignoreCase;
    }

    /**
     * Compare digits at certain position in two strings.
     * The longest run of digits wins. That aside, the greatest
     * value wins.
     */
    protected int compareDigits(String str1, int ndx1, String str2, int ndx2) {
        int bias = 0;

        while (true) {
            char char1 = charAt(str1, ndx1);
            char char2 = charAt(str2, ndx2);

            boolean isDigitChar1 = CharUtil.isDigit(char1);
            boolean isDigitChar2 = CharUtil.isDigit(char2);

            if (!isDigitChar1 && !isDigitChar2) {
                return bias;
            }
            if (!isDigitChar1) {
                return -1;
            }
            if (!isDigitChar2) {
                return 1;
            }

            if (char1 < char2) {
                if (bias == 0) {
                    bias = -1;
                }
            } else if (char1 > char2) {
                if (bias == 0) {
                    bias = 1;
                }
            } else if (char1 == 0 && char2 == 0) {
                return bias;
            }

            ndx1++;
            ndx2++;
        }
    }

    public int compare(T o1, T o2) {
        String str1 = o1.toString();
        String str2 = o2.toString();

        int ndx1 = 0, ndx2 = 0;
        int zeroCount1, zeroCount2;
        char char1, char2;

        int result;

        while (true) {
            // only count the number of zeroes leading the last number compared
            zeroCount1 = zeroCount2 = 0;

            char1 = charAt(str1, ndx1);
            char2 = charAt(str2, ndx2);

            // skip over leading spaces or zeros in both strings

            while (Character.isSpaceChar(char1) || char1 == '0') {
                if (char1 == '0') {
                    zeroCount1++;
                } else {
                    zeroCount1 = 0; // counts only last 0 prefixes, space char interrupts the array of 0s
                }
                ndx1++;
                char1 = charAt(str1, ndx1);
            }

            while (Character.isSpaceChar(char2) || char2 == '0') {
                if (char2 == '0') {
                    zeroCount2++;
                } else {
                    zeroCount2 = 0;
                }
                ndx2++;
                char2 = charAt(str2, ndx2);
            }

            // process digits

            boolean isDigitChar1 = CharUtil.isDigit(char1);
            boolean isDigitChar2 = CharUtil.isDigit(char2);

            if (isDigitChar1 && isDigitChar2) {
                result = compareDigits(str1, ndx1, str2, ndx2);
                if (result != 0) {
                    // not equals, return
                    return result;
                }
                // equal numbers
                if (zeroCount1 != zeroCount2) {
                    return zeroCount1 - zeroCount2;
                }
            }

            if (char1 == 0 && char2 == 0) {
                // the end; the strings are the same, maybe compare ascii?
                return zeroCount1 - zeroCount2;
            }

            // check when one of the numbers is just zeros
            if (isDigitChar1 || isDigitChar2) {
                if (zeroCount1 != zeroCount2) {
                    return zeroCount2 - zeroCount1;
                }
            }

            // checks when both numbers are zero
            if (zeroCount1 != zeroCount2) {
                return zeroCount1 - zeroCount2;
            }

            // compare chars
            if (ignoreCase) {
                char1 = Character.toLowerCase(char1);
                char2 = Character.toLowerCase(char2);
            }
            if (char1 < char2) {
                return -1;
            }
            if (char1 > char2) {
                return 1;
            }

            ndx1++;
            ndx2++;
        }
    }

    /**
     * Safe charAt.
     */
    private static char charAt(String s, int i) {
        if (i >= s.length()) {
            return 0;
        }
        return s.charAt(i);
    }
}

throws the exception:

java.lang.IllegalArgumentException: Comparison method violates its general contract!
    at java.util.TimSort.mergeLo(TimSort.java:747)
    at java.util.TimSort.mergeAt(TimSort.java:483)
    at java.util.TimSort.mergeCollapse(TimSort.java:410)
    at java.util.TimSort.sort(TimSort.java:214)
    at java.util.TimSort.sort(TimSort.java:173)
    at java.util.Arrays.sort(Arrays.java:659)
    at java.util.Collections.sort(Collections.java:217)

This is called by the following function:

@Override
    public int compare(final T o1, final T o2) {
        int result;
        final MyObject obj1 = (MyObject) o1;
        final MyObject obj2 = (MyObject) o2;

                   return     result = compareStringId(obj1.getStringId(),obj2.getStringId());           

    }

private int compareStringId(final String Id1, final String Id2) {
        return super.compare((T) Id1, (T) Id2);
    }

It work fine on our local machines, however it fails on production and we have no means to connect to that machine to figure out why. Could you please assist

igr
  • 10,199
  • 13
  • 65
  • 111
TheHost
  • 15
  • 8
  • Does this help? http://stackoverflow.com/questions/19325256/java-lang-illegalargumentexception-comparison-method-violates-its-general-contr – ppasler Jan 13 '17 at 12:29
  • 1
    What's the super class that `super.compare((T)...)` refers to? And why do you cast the strings to `T`? Btw, which Java version do you use locally and on your server? If your code works locally you're probably running a different version which somehow misses that `ClassCastException` which is probably thrown when casting `String` to `T`. – Thomas Jan 13 '17 at 12:30
  • Another question: you posted some comparator code but you didn't show how you are calling that (i.e. what code calls `Collections.sort(...)`). – Thomas Jan 13 '17 at 12:36
  • 1
    If `a.compareTo(b)` returns -1, then `b.compareTo(a)` must return 1. And if `a.compareTo(b)` returns 0, then `b.compareTo(a)` must also return 0. Your compareTo method does not keep this contract. The reason why you don't get the exception on your local system is probably because you are using a java version <1.7, where MergeSort was still the default algorythm and not TimSort. I recommend putting in loggers so that you can see what values your compareTo method returns for what passed arguments. That should help you find the problem in your compare logic – OH GOD SPIDERS Jan 13 '17 at 12:44
  • 1
    It would be super-cool if you figure what are the input data that failed this comparator on Java8... and report to Jodd issues on GitHub. Im trying to reproduce, so far no luck – igr Jan 15 '17 at 06:39
  • If you find the input that fails, please report it [here](https://github.com/oblac/jodd/issues/381) – igr Jan 16 '17 at 23:34
  • You can go through answers of similar question http://stackoverflow.com/questions/8327514/comparison-method-violates-its-general-contract – Anil Agrawal Jan 20 '17 at 09:37
  • Possible duplicate of [Java error: Comparison method violates its general contract](http://stackoverflow.com/questions/11441666/java-error-comparison-method-violates-its-general-contract) – Raedwald Jan 20 '17 at 09:43
  • Could you share more of your `Comparator` class (the one that contains `compareStringId`)? – Thomas Kläger Jan 22 '17 at 16:31

1 Answers1

2

The issue was with the incorrect implementation of a Comparator. According to Java documentation, a Comparator must be both reflexive and transitive. In this case, the transivity was not guaranteed. Prior Java 8 that was not a big issue, i.e. the sorting implementation (MergeSort) would not throw exception. Java8 changed default sorting implementation to TimSort that is much more sensitive to comparators with invalid contract, hence it might throw an exception.

However, this does not help you much in solving your issue. How about to check the latest version of the same class here. It has been upgraded to support the comparator contract, plus it works better on some edge cases, not to mention initial support for accents.

igr
  • 10,199
  • 13
  • 65
  • 111