Texas Hold em Poker Hand recognition algorithm and implementation

I am designing an in-depth poker game. I am first focusing on recognizing the strength of a hand given the set of cards.

• Is the following algorithm suitable for the stated purpose?

• Am I using correct OOP design principles and implementation in my code?

Algorithm:

Go from a top down approach checking for the following in order:

• Is: the hand a royal flush

• If not: is the hand a straight flush

• If not: is the hand a four of a kind

• If not: is the hand a full house

Eventually if none are met, it will just return the high card.

Checking for each individual instance:

• Royal flush: is it a flush and is it a straight and are all the cards picture cards

• Is a straight flush: is it a straight and is it a flush

• Is it a 4 of a kind: is the same card repeated 4 times

• Is it a full house: is there a 3 of a kind and a 2 of a kind

• Is it a flush: are there 5 cards with the same suit

• Is it a straight: are there 5 cards in a row with a common difference of 1

• Is it a 3 of a kind: are there 3 repeated cards

• Is it a two pair: are there two pairs

• Is it a pair: are there 2 repeated cards

```
package main;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.stream.Stream;

/**
*
* @author Tamir
*/
public class Hand {

private Card[] hand = new Card[2];

public enum HandRank {

ROYAL_FLUSH,
STRAIGHT_FLUSH,
FOUR_OF_A_KIND,
FULL_HOUSE,
FLUSH,
STRAIGHT,
THREE_OF_A_KIND,
TWO_PAIR,
PAIR,
HIGH_CARD;
}

public Hand() {
}

public Hand(Card[] hand) {
this.hand = hand;
}

public Card[] getHand() {
return hand;
}

public void setHand(Card[] hand) {
this.hand = hand;
}

public void printHand() {
for (Card c : hand) {
System.out.println(c);
}
}

public HandRank determineHandRank(Card[] flop) {
if (isARoyalFlush(flop)) {
return HandRank.ROYAL_FLUSH;
} else if (isAStraightFlush(flop)) {
re

Good job! Just some points:

Bugs

• Royal Flush Checking - 10 S, J S, Q S, K S, 2 H as flop and 8 S, A S in the hand will return false if passed through isARoyalFlush(). What??? You seem to only be checking for Straights and Flushes with the flop.

• Flush checking - If there is 6 Spades, or 6 Hearts or 6/7 whatever, then the isAFlush() method returns false. Change:

return (noOfClubs == 5 || noOfSpades == 5 || noOfHearts == 5 || noOfDiamonds == 5);

To:

return (noOfClubs >= 5 || noOfSpades >= 5 || noOfHearts >= 5 || noOfDiamonds >= 5);

-
Straight Flush Checking - Two things gone wrong here.

a) You're only checking for a Straight Flush in the flop. What if the Hand contributes to a Straight Flush?

b) Don't feel bad about this one too much; this is a common mistake that even I made when writing a Hand Evaluator (I figured it out and decided it was way too hard; so I gave up). 3 S, 4 S, 5 S, 6 S, 10 S as flop and 8 H, 7 H in the hand will return true (after you edit it so that it checks both the hand and the flop), even though you see that there is no Straight Flush.

Naming

public boolean isARoyalFlush(Card[] flop)

Why the "A" in the middle? I would remove it completely, as it both reduces readability and is doesn't add anything to the meaning.

Same with all the other ones.

In isARoyalFlush(Card[] flop)

switch (c.getRank().getRank())

What??? .getRank().getRank() confuses me. Might want to do some naming changes. I would simply remove getRank() in Rank and use toString() instead:

@Override
public String toString() {
    return rank;
}

And maybe even change the naming for rank.

Also, checking for Straight beforehand is not necessary, as your code checks if it is a flush and later checks if it contains 10, J, Q, K, and A anyways. isStraight() will take about the same time to run as isRoyalFlush(), making the possible performance gain from a false evaluation not worth it.

Others

In isAStraightFlush(Card[] flop)

private boolean isAStraightFlush(Card[] flop) {
    if (isAFlush(flop) && isAStraight(flop)) {
        return true;
    } else {
        return false;
    }
}

That could easily be:

private boolean isAStraightFlush(Card[] flop) {
    return isAFlush(flop) && isAStraight(flop);
}

Though, as mentioned in the Bugs section, it doesn't really work.

In isThreeOfAKind(Card[] flop)

Hmm... Here you don't do isAThreeOfAKind(Card[] flop)...

Also, here:

while (i < allCards.length && !isThreeOfAKind) {
    cardRepeats = 1;
    while (k < allCards.length && !isThreeOfAKind) {
        if (allCards[i].getRank().getValue() == allCards[k].getRank().getValue()) {
            cardRepeats++;
            if (cardRepeats == 3) {
                isThreeOfAKind = true;
            }
        }
        k++;
    }
    i++;
}
return isThreeOfAKind;

You can just as easily return in the inner if statement immediately, without going through the checks in the loop:

while (i < allCards.length) {
    cardRepeats = 1;
    while (k < allCards.length) {
        if (allCards[i].getRank().getValue() == allCards[k].getRank().getValue()) {
            cardRepeats++;
            if (cardRepeats == 3) {
                return true;
            }
        }
        k++;
    }
    i++;
}
return false;

In isTwoPair(Card[] flop)

See all advice in isThreeOfAKind(Card[] flop).

In isPair(Card[] flop)

Again, see all advice in isThreeOfAKind(Card[] flop).

In isAFourOfAKind(Card[] flop)

Not the A again...

See second piece of advice in isThreeOfAKind(Card[] flop).

Good Job!

Hand Evaluators are very hard to implement. It's very courageous of you to tackle such a challenging task (as you can see, there are quite a few bugs). Good luck improving your code, and I hope to see a follow-up after the bugs are fixed!

StackExchange Code Review Q#91086, answer score: 4