1

I am aware of the compare rules in Java (and in general) as described here.
I have an ArrayList of Strings.
Each String represent a Texas Hold'em Poker Hand ignoring the suit.
Each String is exactly 13 characters long.
Each String consists of digits only whose sum is 7.

For example, "0100300200100" represent a Poker Hand which consists of one card of 3, three cards of 6, two cards of 9 and one card of Queen.
(In this case the hand represent a Full House - sixes full of nines).

I want to sort this list according to the strength of the Poker hands.
I have the following java code who implements the Comparator's compare method.

final Comparator<String> COMBINATION_ORDER = new Comparator<String>() {
    @Override
    public int compare(String c1, String c2) {
        if (c1.indexOf('4') != -1 || c2.indexOf('4') != -1) {  // Four of a kind
            if (c1.indexOf('4') == c2.indexOf('4')) {
                for (int i = 12; i >= 0; i--) {
                    if (c1.charAt(i) != '0' && c1.charAt(i) != '4') {
                        if (c2.charAt(i) != '0' && c2.charAt(i) != '4') {
                            return 0;
                        }
                        return 1;
                    }
                    if (c2.charAt(i) != '0' && c2.charAt(i) != '4') {
                        return -1;
                    }
                }
            }
            return c1.indexOf('4') - c2.indexOf('4');
        }
        int tripleCount1 = StringFunctions.countOccurrencesOf(c1, "3");
        int tripleCount2 = StringFunctions.countOccurrencesOf(c2, "3");
        if (tripleCount1 > 1 || (tripleCount1 == 1 && c1.indexOf('2') != -1) || tripleCount2 > 1 || (tripleCount2 == 1 && c2.indexOf('2') != -1)) {  // Full house
            int higherTriple = c1.lastIndexOf('3');
            if (higherTriple == c2.lastIndexOf('3')) {
                for (int i = 12; i >= 0; i--) {
                    if (i == higherTriple) {
                        continue;
                    }
                    if (c1.charAt(i) == '2' || c1.charAt(i) == '3') {
                        if (c2.charAt(i) == '2' || c2.charAt(i) == '3') {
                            return 0;
                        }
                        return 1;
                    }
                    if (c2.charAt(i) == '2' || c2.charAt(i) == '3') {
                        return -1;
                    }
                }
            }
            return higherTriple - c2.lastIndexOf('3');
        }
        return 0;
    }
};

In the meantime I refer only to Four-of-a-Kind and Full House. That means that every other hand will be treated as equal to each other (but inferior to Four-of-a-Kind or Full House).

But when I'm sorting:

combinations.sort(COMBINATION_ORDER);

(where combinations is my ArrayList).

I get an Exception.

Exception in thread "main" java.lang.IllegalArgumentException: Comparison method violates its general contract!
    at java.util.TimSort.mergeLo(TimSort.java:773)
    at java.util.TimSort.mergeAt(TimSort.java:510)
    at java.util.TimSort.mergeCollapse(TimSort.java:437)
    at java.util.TimSort.sort(TimSort.java:241)
    at java.util.Arrays.sort(Arrays.java:1512)
    at java.util.ArrayList.sort(ArrayList.java:1454)
    at Poker.main(Poker.java:120)

Please help me understand what's wrong with the code.
Thanks very much.

EDIT:

As @ajb said I didn't take into consideration Three-of-a-Kind that has no Full House.

SOLUTION:

final Comparator<String> COMBINATION_ORDER = new Comparator<String>() {
    @Override
    public int compare(String c1, String c2) {
        if (c1.indexOf('4') != -1 || c2.indexOf('4') != -1) {  // Four of a kind
            if (c1.indexOf('4') == c2.indexOf('4')) {
                for (int i = 12; i >= 0; i--) {
                    if (c1.charAt(i) != '0' && c1.charAt(i) != '4') {
                        if (c2.charAt(i) != '0' && c2.charAt(i) != '4') {
                            return 0;
                        }
                        return 1;
                    }
                    if (c2.charAt(i) != '0' && c2.charAt(i) != '4') {
                        return -1;
                    }
                }
            }
            return c1.indexOf('4') - c2.indexOf('4');
        }
        int tripleCount1 = StringFunctions.countOccurrencesOf(c1, "3");
        int tripleCount2 = StringFunctions.countOccurrencesOf(c2, "3");
        if (tripleCount1 > 1 || (tripleCount1 == 1 && c1.indexOf('2') != -1)) {      // c1 Full house
            if (tripleCount2 > 1 || (tripleCount2 == 1 && c2.indexOf('2') != -1)) {  // c2 Full house too
                int higherTriple = c1.lastIndexOf('3');
                if (higherTriple == c2.lastIndexOf('3')) {
                    for (int i = 12; i >= 0; i--) {
                        if (i == higherTriple) {
                            continue;
                        }
                        if (c1.charAt(i) == '2' || c1.charAt(i) == '3') {
                            if (c2.charAt(i) == '2' || c2.charAt(i) == '3') {
                                return 0;
                            }
                            return 1;                                                // only c1 Full house
                        }
                        if (c2.charAt(i) == '2' || c2.charAt(i) == '3') {            // only c2 Full house
                            return -1;
                        }
                    }
                }
                return higherTriple - c2.lastIndexOf('3');
            }
            return 1;
        }
        if (tripleCount2 > 1 || (tripleCount2 == 1 && c2.indexOf('2') != -1)) {
            return -1;
        }
        return 0;
    }
};
LionGod8
  • 33
  • 1
  • 5
  • possible duplicate of [this question](http://stackoverflow.com/q/11441666/524900)? – Lolo May 26 '15 at 04:40
  • So how are you going to detect a flush? :) – ajb May 26 '15 at 04:43
  • Without looking at it too hard, I think your logic is faulty. If `(c1.indexOf('4') == c2.indexOf('4'))` is true, then you have a deck with eight of the same card in it. That doesn't explain the exception, but it makes me think that your algorithm may have other errors in it. – ajb May 26 '15 at 04:49
  • @ajb, to detect a Flush I use another code. This code is used when I certainly know there is no Flush. – LionGod8 May 26 '15 at 05:46

1 Answers1

2

One of the conditions your comparator must obey is that it must be transitive. That is, if A > B and B > C, then A > C. If the comparator doesn't follow this rule, the sort may run into a case where the ordering is not what it expects, and then it will throw an exception.

There's at least one logic error in your algorithm. (There may be other errors, but I can definitely spot this one and it can definitely cause the exception.) The problem is when one hand has a full house, and the other hand has 3 of a kind but not a full house. Your code does not always make the full house greater. If the 3-of-a-kind is three of a higher card than the three cards in the full house, the 3-of-a-kind will compare greater. So say one hand is KKK8743, another is QQQ6632, and another is JJJ8743. Your code erroneously makes KKK8743 > QQQ6632. It also says QQQ6632 > JJJ8743. But it also says KKK8743 = JJJ8743, so transitivity is violated.

ajb
  • 31,309
  • 3
  • 58
  • 84