1

I have tried many possible solution given on the net like to set System property and to convert in double but still getting same error:

java.lang.IllegalArgumentException: Comparison method violates its general contract!
    at java.util.ComparableTimSort.mergeHi(ComparableTimSort.java:835)
    at java.util.ComparableTimSort.mergeAt(ComparableTimSort.java:453)
    at java.util.ComparableTimSort.mergeForceCollapse(ComparableTimSort.java:392)
    at java.util.ComparableTimSort.sort(ComparableTimSort.java:191)
    at java.util.ComparableTimSort.sort(ComparableTimSort.java:146)
    at java.util.Arrays.sort(Arrays.java:472)
    at java.util.Collections.sort(Collections.java:155)

Here is my code:

        System.setProperty("java.util.Arrays.useLegacyMergeSort", "true");
        Collections.sort(docs, new Comparator<FeedDocument>() {
            public int compare(FeedDocument o1, FeedDocument o2) {

                int year1 = 0;
                int year2 = 0;
                int returnResult = 0;
                if (o1.containsKey(FeedConstants.PUBLICATION_YEAR)
                        && o2.containsKey(FeedConstants.PUBLICATION_YEAR)
                        && o1.get(FeedConstants.PUBLICATION_YEAR) != null
                        && (o1.get(FeedConstants.PUBLICATION_YEAR) instanceof String)
                        && o2.get(FeedConstants.PUBLICATION_YEAR) != null
                        && (o2.get(FeedConstants.PUBLICATION_YEAR) instanceof String)) {

                    String firstyear = (String) o1.get((FeedConstants.PUBLICATION_YEAR));
                    String secondyear = (String) o2.get((FeedConstants.PUBLICATION_YEAR));

                    if (firstyear.equals(secondyear)) {
                        return 0;
                    } else if (firstyear != null && !firstyear.isEmpty() && secondyear != null
                            && !secondyear.isEmpty()) {

                        year1 = Integer.parseInt(firstyear.trim());

                        year2 = Integer.parseInt(secondyear.trim());

                        // int result = year2 - year1;
                        // if (result > 0) {
                        // returnResult = 1;
                        // } else if (result < 0) {
                        // returnResult = -1;
                        // }
                        return Double.compare(year2, year1);
                    }

                } else {
                    returnResult = 0;
                }
                return returnResult;
            }
        });
azurefrog
  • 10,785
  • 7
  • 42
  • 56
Tanu Garg
  • 3,007
  • 4
  • 21
  • 29
  • 1
    Possible duplicate of ["Comparison method violates its general contract!"](http://stackoverflow.com/questions/8327514/comparison-method-violates-its-general-contract) – bradimus Aug 17 '16 at 20:17
  • Just FYI `instanceof` returns `false` if its left argument is null, no need for another check. – mszymborski Aug 17 '16 at 20:17
  • Yeah, you can combine `o1.containsKey(FeedConstants.PUBLICATION_YEAR) && o1.get(FeedConstants.PUBLICATION_YEAR) != null && o1.get(FeedConstants.PUBLICATION_YEAR) instanceof String` into simply `o1.get(FeedConstants.PUBLICATION_YEAR) instanceof String`. You also have additional unnecessary checks down the road: `firstyear != null` and `secondyear != null`. These just clutter the code and make it harder to follow. – mapeters Aug 18 '16 at 02:42

2 Answers2

4

Pretty sure I know what's happening here...

Suppse: 

o1.get(FeedConstants.PUBLICATION_YEAR) != null
o2.get(FeedConstants.PUBLICATION_YEAR) == null
o3.get(FeedConstants.PUBLICATION_YEAR) != null

Then:

compare (o1, o2); //returns 0
compare (o2, o3); //returns 0
compare (o1, o3); //returns not 0

So you're claiming o1 == o2 == o3 but o1 != o3

Edward Peters
  • 3,623
  • 2
  • 16
  • 39
  • @TanuGarg You just have to make it so a single null value doesn't return equality. The standard I generally use is "If they're both null they're equal, if one is null it's automatically lesser, if neither are null compare normally." – Edward Peters Aug 18 '16 at 18:27
0

Edward Peters' answer is correct in diagnosing your problem, as your compare method doesn't produce consistent (transitive) results.

The best way to resolve this is something like the following in your compare method:

if (o1.get(FeedConstants.PUBLICATION_YEAR) instanceof String) {
    if (o2.get(FeedConstants.PUBLICATION_YEAR) instanceof String) {
        // Perform the comparison here like you are
    } else {
        /*
         * This could also be 1, the key is to have it consistent
         * so the final sorted list clearly separates the FeedDocuments
         * with a String PUBLICATION_YEAR and those without one.
         */
        return -1;
    }
} else if (o2.get(FeedConstants.PUBLICATION_YEAR) instanceof String) {
    /*
     * In this case, o1 doesn't have a String PUBLICATION_YEAR and o2
     * does, so this needs to be the opposite of the return value
     * 6 lines up to be consistent.
     */
     return 1;
} else {
     /*
      * Consider all FeedDocuments without a String PUBLICATION_YEAR
      * to be equivalent, otherwise you could do some other comparison
      * on them here if you wanted.
      */
     return 0;
}

The key is, if you only care about a subset of the list being sorted (the FeedDocuments with a String publication year, then you need to first separate them from the rest of the list that you don't care about being sorted (by returning either 1 or -1 when one of the FeedDocuments has a String publication year and the other doesn't). Then, you are free to sort the desired subset without inconsistent results.

Community
  • 1
  • 1
mapeters
  • 1,067
  • 7
  • 11
  • I have not understood /* * In this case, o1 doesn't have a String PUBLICATION_YEAR and o2 * does, so this needs to be the opposite of the return value * 6 lines up to be consistent. */ – Tanu Garg Aug 18 '16 at 18:24
  • @TanuGarg at the `return -1` line, we have determined that `o1` has a publication year, and that `o2` does not. So, `return -1` says that `o1` (and all `FeedDocument`s with a publication year) should come before those that don't have a publication year. At the `return 1` line, we have determined that `o1` doesn't have a publication year, but `o2` does, which is the opposite scenario of the first. So, saying `return 1` puts `o1` (and all `FeedDocument`s without a publication year) after those that have a publication year, which matches what happened on the `return -1` line. – mapeters Aug 18 '16 at 18:33
  • 1
    Understood. Thanks for explanation also error has been resolved. – Tanu Garg Aug 18 '16 at 18:42
  • @TanuGarg no problem, glad to help! – mapeters Aug 18 '16 at 18:47