8
\$\begingroup\$

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

  1. Is the following algorithm suitable for the stated purpose?
  2. 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:

  1. Is: the hand a royal flush
  2. If not: is the hand a straight flush
  3. If not: is the hand a four of a kind
  4. 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:

  1. Royal flush: is it a flush and is it a straight and are all the cards picture cards
  2. Is a straight flush: is it a straight and is it a flush
  3. Is it a 4 of a kind: is the same card repeated 4 times
  4. Is it a full house: is there a 3 of a kind and a 2 of a kind
  5. Is it a flush: are there 5 cards with the same suit
  6. Is it a straight: are there 5 cards in a row with a common difference of 1
  7. Is it a 3 of a kind: are there 3 repeated cards
  8. Is it a two pair: are there two pairs
  9. 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)) {
        return HandRank.STRAIGHT_FLUSH;

    } else if (isAFourOfAKind(flop)) {
        return HandRank.FOUR_OF_A_KIND;
    } else if (isAFullHouse(flop)) {
        return HandRank.FULL_HOUSE;
    } else if (isAFlush(flop)) {
        return HandRank.FLUSH;
    } else if (isAStraight(flop)) {
        return HandRank.STRAIGHT;
    } else if (isThreeOfAKind(flop)) {
        return HandRank.THREE_OF_A_KIND;
    } else if (isTwoPair(flop)) {
        return HandRank.TWO_PAIR;
    } else if (isPair(flop)) {
        return HandRank.PAIR;
    } else {
        return HandRank.HIGH_CARD;
    }

}

public boolean isARoyalFlush(Card[] flop) {
    if (isAStraight(flop) && isAFlush(flop)) {
        Card[] allCards = Stream.concat(Arrays.stream(flop), Arrays.stream(hand))
                .toArray(Card[]::new);
        boolean aceExists = false, kingExists = false, queenExists = false, jackExists = false, tenExists = false;
        for (Card c : allCards) {
            switch (c.getRank().getRank()) {
                case "ACE":
                    aceExists = true;
                    break;
                case "KING":
                    kingExists = true;
                    break;
                case "QUEEN":
                    queenExists = true;
                    break;
                case "JACK":
                    jackExists = true;
                    break;
                case "TEN":
                    tenExists = true;
                    break;

            }
        }
        return (aceExists && kingExists && queenExists && jackExists && tenExists);
    } else {
        return false;
    }
}

public boolean isAStraight(Card[] flop) {
    Card[] allCards = Stream.concat(Arrays.stream(flop), Arrays.stream(hand))
            .toArray(Card[]::new);
    Arrays.sort(allCards, byRank);
    int noOfCardsInARow = 0;
    int pos = 0;
    boolean isAStraight = false;
    while (pos < allCards.length - 1 && !isAStraight) {
        if (allCards[pos + 1].getRank().getValue() - allCards[pos].getRank().getValue() == 1) {
            noOfCardsInARow++;
            if (noOfCardsInARow == 4) {
                isAStraight = true;
            } else {
                pos++;
            }
        } else {
            noOfCardsInARow = 0;
            pos++;
        }
    }
    return isAStraight;
}

public boolean isAFlush(Card[] flop) {
    Card[] allCards = Stream.concat(Arrays.stream(flop), Arrays.stream(hand))
            .toArray(Card[]::new);
    int noOfClubs = 0;
    int noOfSpades = 0;
    int noOfHearts = 0;
    int noOfDiamonds = 0;
    for (Card c : allCards) {
        switch (c.getSuit()) {
            case "HEART":
                noOfHearts++;
                break;
            case "SPADES":
                noOfSpades++;
                break;
            case "CLUBS":
                noOfClubs++;
                break;
            case "DIAMONDS":
                noOfDiamonds++;
                break;
        }
    }
    return (noOfClubs == 5 || noOfSpades == 5 || noOfHearts == 5 || noOfDiamonds == 5);
}

private boolean isThreeOfAKind(Card[] flop) {
    Card[] allCards = Stream.concat(Arrays.stream(flop), Arrays.stream(hand))
            .toArray(Card[]::new);
    int cardRepeats = 1;
    boolean isThreeOfAKind = false;
    int i = 0;
    int k = i + 1;
    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;
}

private boolean isTwoPair(Card[] flop) {
    Card[] allCards = Stream.concat(Arrays.stream(flop), Arrays.stream(hand))
            .toArray(Card[]::new);
    int cardRepeats = 1;
    int noOfCardRepeats = 0;
    boolean isTwoPair = false;
    int i = 0;
    int k = i + 1;
    while (i < allCards.length && !isTwoPair) {
        cardRepeats = 1;
        while (k < allCards.length && !isTwoPair) {
            if (allCards[i].getRank().getValue() == allCards[k].getRank().getValue()) {
                cardRepeats++;
                if (cardRepeats == 2) {
                    cardRepeats = 1;
                    noOfCardRepeats++;
                    if (noOfCardRepeats == 2) {
                        isTwoPair = true;

                    }
                }

            }
            k++;
        }
        i++;
    }
    return isTwoPair;
}

private boolean isPair(Card[] flop) {
    Card[] allCards = Stream.concat(Arrays.stream(flop), Arrays.stream(hand))
            .toArray(Card[]::new);
    int cardRepeats = 1;
    boolean isPair = false;
    int i = 0;
    int k = i + 1;
    while (i < allCards.length && !isPair) {
        cardRepeats = 1;
        while (k < allCards.length && !isPair) {
            if (allCards[i].getRank().getValue() == allCards[k].getRank().getValue()) {
                cardRepeats++;
                if (cardRepeats == 2) {
                    isPair = true;
                }
            }
            k++;
        }
        i++;
    }
    return isPair;
}
public Comparator<Card> byRank = (Card left, Card right) -> {
    if (left.getRank().getValue() < right.getRank().getValue()) {
        return -1;
    } else {
        return 1;
    }
};

private boolean isAFullHouse(Card[] flop) {
    Card[] allCards = Stream.concat(Arrays.stream(flop), Arrays.stream(hand))
            .toArray(Card[]::new);

    Arrays.sort(allCards, byRank);
    int noOfRepeats = 1;
    boolean isThreeOfAKind = false;
    boolean isTwoOfAKind = false;
    for (int i = 0; i < allCards.length - 1; i++) {
        if (allCards[i].getRank().getValue() == allCards[i + 1].getRank().getValue()) {
            noOfRepeats++;
            if (noOfRepeats == 3) {
                isThreeOfAKind = true;
                noOfRepeats = 1;
            } else if (noOfRepeats == 2) {
                isTwoOfAKind = true;
                noOfRepeats = 1;
            }
        } else {
            noOfRepeats = 1;
        }
    }
    return (isTwoOfAKind && isThreeOfAKind);

}

public boolean isAFourOfAKind(Card[] flop) {
    Card[] allCards = Stream.concat(Arrays.stream(flop), Arrays.stream(hand))
            .toArray(Card[]::new);
    int cardRepeats = 1;
    boolean isFourOfAKind = false;
    int i = 0;
    int k = i + 1;
    while (i < allCards.length && !isFourOfAKind) {
        cardRepeats = 1;
        while (k < allCards.length && !isFourOfAKind) {
            if (allCards[i].getRank().getValue() == allCards[k].getRank().getValue()) {
                cardRepeats++;
                if (cardRepeats == 4) {
                    isFourOfAKind = true;
                }
            }
            k++;
        }
        i++;
    }
    return isFourOfAKind;
}

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

public Card getHighCard(Card[] flop) {
    Card[] allCards = Stream.concat(Arrays.stream(flop), Arrays.stream(hand))
            .toArray(Card[]::new);
    Arrays.sort(allCards, byRank);
    return allCards[0];
}

public Card getHandHighCard() {
    Arrays.sort(hand, byRank);
    return hand[0];
}
}

Card object

/*
 * To change this license header, choose License Headers in Project    Properties.
 * To change this template file, choose Tools | Templates
 * and open the template in the editor.
 */
package main;

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

private String suit;
private Rank rank;
//isDealt: checks if the card is in play and out of the deck
private boolean isDealt;

public Card() {

}

public Card(String suit, Rank rank, boolean isDealt) {
    this.suit = suit;
    this.rank = rank;
    this.isDealt = isDealt;
}

public String getSuit() {
    return suit;
}

public void setSuit(String suit) {
    this.suit = suit;
}

public Rank getRank() {
    return rank;
}

public void setRank(Rank rank) {
    this.rank = rank;
}

public boolean isIsDealt() {
    return isDealt;
}

public void setIsDealt(boolean isDealt) {
    this.isDealt = isDealt;
}

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

}

Rank object

package main;

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

private int value;
private String rank;

public Rank() {
}

public Rank(int value, String rank) {
    this.value = value;
    this.rank = rank;
}

public int getValue() {
    return value;
}

public void setValue(int value) {
    this.value = value;
}

public String getRank() {
    return rank;
}

public void setRank(String rank) {
    this.rank = rank;
}

@Override
public String toString() {
    String stg = "";
    stg += this.rank + "(" + this.value + ")";
    return stg;
}

}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Welcome to Code Review! You seem to be missing a closing brace. Also, to make your code easier to review, could you post your Card and Rank classes too? \$\endgroup\$ May 18, 2015 at 18:57
  • \$\begingroup\$ TwoPair is wrong, in my opinion. What if the allCards array is [2,2,3,2]? You'll get a two pair, but it obviously isn't one. \$\endgroup\$
    – user85946
    Oct 2, 2015 at 23:49

3 Answers 3

4
\$\begingroup\$

Good job! Just some points:

Bugs

  1. 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.
  2. 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);
  1. 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!

\$\endgroup\$
2
  • \$\begingroup\$ Thank you so so much!I cannot begin to describe how helpful this is. Just a general question after I make revisions to my code must I post a new thread or simply update this one? \$\endgroup\$ May 23, 2015 at 17:20
  • 1
    \$\begingroup\$ @TamirShklaz You must post a new question. \$\endgroup\$ May 24, 2015 at 18:10
1
\$\begingroup\$

I did something like this (in C++), and it would be more easy if you order the board cards by their index, internally. So, if the board is Ax Kx Tx Jx Qx then, if you operate on the "behind" vector of cards [ordered by index, as in TJQKA] you can "see" more easily that this is a straight (the difference between the "behind" vector's indexes is one and the last one is an Ace ==> this is a straight); it is also MUCH more easy to "compute" the 1,2 pairs, 3,4 of a kind... Cheers!

\$\endgroup\$
1
\$\begingroup\$

Performance

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.

You could perform an optimized top down evaluation. In text and pseudo-code:

  1. Evaluate both for straight and flush -> cache your results
  2. Evaluate for straight flush given the results from (1) if straight & flush
  3. Evaluate for royal flush given the results from (2) if straight flush & high ace
  4. Perform regular top down evaluations, but for any of the results already cached in (1), (2) or (3) return the cached result if !cached then evaluate
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.