3
\$\begingroup\$

If you don't play poker this will be hard to follow.

In Hold'em and other Poker games you take the best 5 of 7.

The class Hand selects the best 5 and scores the hand. Hand implements IComparable so hands may be compared. In simulations will create millions of boards to compare hands so speed is a priority.

There are 133784560 unique combinations of 7 cards from 52. When compare this to count of hand ranks from here it is correct.

I have run 4 different hand to hand match ups and gotten the same result as on line tools.

I chose to score the hand in the constructor as every hand needs to get scored. Report the best 5 cards in addition is a lot of overhead and not really required for a most simulations but used in a game play application then need the hand.

A direct hand to hand comparison and just hole cards hero, hole cards villain, and 5 cards on the board would be faster as you don't need to get into scoring the individual cards if the hand ranks are different.

public enum HandRank { high = 0, onePr = 1, twoPr = 2, trip = 3, straight = 4, flush = 5, boat = 6, quad = 7, strFlush = 8  }
public enum CardRank { two = 0, three = 1, four = 2, five = 3, six = 4, seven = 5, eight = 6, nine = 7, ten = 8, jack = 9, queen = 10, king = 11, ace = 12 }
public enum CardSuit { spade = 0, club = 1, heart = 2, diamond = 3 }
class Card : IComparable
{
    public override bool Equals(Object obj)
    {
        // Check for null values and compare run-time types.
        if (obj == null || GetType() != obj.GetType())
            return false;

        Card c = (Card)obj;
        return (this.Quick == c.Quick);
    }
    public override int GetHashCode()
    {
        return this.Quick;
    }
    public int CompareTo(Object obj)
    {
        if (obj == null) return 1;

        Card other = (Card)obj;
        if (other != null)
            return this.RankByte.CompareTo(other.RankByte);
        else
            throw new ArgumentException("Object is not a Card");
    }
    public Card(byte Rank, byte Suit)
    {
        rank = Rank;
        suit = Suit;
        quick = (byte)(13 * suit + rank);
    }
    public Card(byte Quick)
    {
        rank = (byte)(Quick % 13);
        suit = (byte)(Quick / 13);
        quick = Quick;
    }
    public Card(Card card)
    {
        rank = card.RankByte;
        suit = card.SuitByte;
        quick = (byte)(13 * suit + rank);
    }
    public int Quick { get { return quick; } }
    byte rank;
    byte suit;
    byte quick;
    public byte RankByte { get { return rank; } }
    public byte SuitByte { get { return suit; } }
    public CardRank Rank { get { return (CardRank)RankByte; } }
    public CardSuit Suit { get { return (CardSuit)SuitByte; } }
}
class Hand : Object, IComparable 
{
    public override bool Equals(Object obj)
    {
        // Check for null values and compare run-time types.
        if (obj == null || GetType() != obj.GetType())
            return false;

        Hand h = (Hand)obj;
        if (h.GetHashCode() != this.GetHashCode())
            return false;
        return (this.Quick.SequenceEqual(h.Quick));
    }           
    public override int GetHashCode()
    {
        return Cards[0].Quick ^ Cards[1].Quick ^ Cards[2].Quick ^ Cards[3].Quick ^ Cards[4].Quick;
    }
    public int CompareTo(Object obj)
    {
        if (obj == null) return 1;

        Hand other = obj as Hand;
        if (other != null)
            return this.Score.CompareTo(other.Score);
        else
            throw new ArgumentException("Object is not a Hand");
    }
    public IEnumerable<int> Quick { get { return Cards.OrderBy(x => x.Quick).Select(x => x.Quick); } }
    public Int64 Score { get; private set; }
    public String ScoreStr
    {
        get { return Score.ToString("N0", new NumberFormatInfo { NumberGroupSizes = new[] { 2, }, });  }
    }
    // x-55-44-33-22-11
    public List<Card> Cards { get; private set; } // this is best 5 ONLY
    public HandRank HandRank { get; private set; }
    public Hand (List<Card> cards, Int64 score, HandRank handRank)
    {
        if (cards.Count != 5)
            throw new IndexOutOfRangeException("Must pass 5 cards");
        HashSet<Card> hsCards = new HashSet<Card>(cards);
        if (hsCards.Count != cards.Count)
            throw new IndexOutOfRangeException("Cards not unique");
        Score = score;
        Cards = cards;
        HandRank = handRank;
    }
    public Hand(Card[] CardArray, int counter)
    {
        // pass in 7 cards - get best 5 and score
        // all 7 should be unique

        if (CardArray.Length != 7)
            throw new IndexOutOfRangeException("Must pass 7 cards");
        HashSet<Card> hsCards = new HashSet<Card>(CardArray);
        if (hsCards.Count != CardArray.Length)
            throw new IndexOutOfRangeException("Cards not unique");

        if (counter == 0 || counter == 1)
        {                
            //foreach (Card c in CardArray)
            //    Debug.WriteLine("rank {0}  suit {1}", c.Rank, c.Suit);
            //Debug.WriteLine("counter " + counter);
        }

        Score = 0;
        //Score r-55-44-33-22-11
        List<Card> cards5 = new List<Card>();
        HandRank = HandRank.high;

        bool haveStraight = false;
        bool haveFlush = false;
        bool haveStraightFlush = false;
        bool haveRoyalFlush = false;
        bool haveBoat = false;
        bool haveQuad = false;
        byte[] rankArray = new byte[13];
        byte[] rankCount = new byte[5];
        byte flushSuit;
        byte flushCount;
        //int counterStraightAndFlush = 0;
        //int straightCount = 0;
        bool haveAce = false;
        int straightCount2 = 0;
        byte? lastRank = null;
        Card firstCard = null;

        foreach (Card c in CardArray)
            rankArray[c.RankByte]++;

        // this will build up the rank count 
        rankCount[0] = 0;  // none   - not used but it needs to be here to make this work
        rankCount[1] = 0;  // single - not used but it needs to be here to make this work
        rankCount[2] = 0;  // pair
        rankCount[3] = 0;  // trips
        rankCount[4] = 0;  // quad
        for (int c = 0; c <= 12; c++)
            rankCount[rankArray[c]]++;

        if (rankCount[4] == 1)
        {
            haveQuad = true;
            //need to get the quads and the largest 
            int highest = -1;
            for (byte i = 0; i < 13; i++)
            {
                if (rankArray[i] == 4)
                {
                    foreach (Card c in CardArray.Where(x => x.RankByte == i))
                        cards5.Add(c);
                }
                else if (highest < i && rankArray[i] > 0)
                    highest = i;
            }
            cards5.Add(CardArray.FirstOrDefault(x => x.RankByte == highest));
        }
        else if (rankCount[3] == 2 || (rankCount[3] == 1 && rankCount[2] > 0))  // with 7 could have two trips
        {
            haveBoat = true;
            byte highest = 0;
            for (byte i = 0; i < 13; i++)
            {
                if (rankArray[i] == 3 && highest < i)
                    highest = i;
            }
            foreach (Card c in CardArray.Where(x => x.RankByte == highest))
                cards5.Add(c);
            highest = 0;
            for (byte i = 0; i < 13; i++)
            {
                if (i == cards5[0].RankByte)
                    continue;
                if (rankArray[i] >= 2 && highest < i)
                    highest = i;
            }
            foreach (Card c in CardArray.Where(x => x.RankByte == highest))
            {
                cards5.Add(c);
                if (cards5.Count == 5)
                    break;
            }
        }

        if (!haveQuad && !haveBoat && rankCount[2] != 3)  // cannot make a straight or flush with a quad, boat, or 3 pair
        {
            // flush
            flushSuit = 0;
            for (byte f = 0; f <= 3; f++)
            {   // can only have one flush in 7 cards 
                if (haveFlush)
                    break;
                flushCount = 0;
                for (int c = 0; c < 7; c++)
                {
                    if (CardArray[c].SuitByte == f)
                    {
                        flushCount++;
                        if (flushCount >= 5)
                        {
                            flushSuit = f;
                            haveFlush = true;
                            break;
                        }
                    }
                }       
            }

            //straight
            //first ace high
            int highCard = -1;

            for (int s = 12; s >= 4; s--)
            {
                highCard = s;
                haveStraight = rankArray[s] > 0 &&
                                rankArray[s - 1] > 0 &&
                                rankArray[s - 2] > 0 &&
                                rankArray[s - 3] > 0 &&
                                rankArray[s - 4] > 0;
                if (haveStraight)
                    break;
            }
            if (!haveStraight)
            {   /// ace as low
                haveStraight = rankArray[3] > 0 &&
                                rankArray[2] > 0 &&
                                rankArray[1] > 0 &&
                                rankArray[0] > 0 &&
                                rankArray[12] > 0;
                highCard = 3;
            }
            //if(haveStraight)
            //{
            //    cards5.Clear();
            //    cards5.Add(new Card(highCard));
            //}

            if (haveStraight && haveFlush)
            {   // now for the difficult task of straightFlush 
                // this is kind of expensive but at least it does hot happen a lot
                // note in 7 you could have gap and still have a straight xyxxxxx

                haveAce = false;
                straightCount2 = 0;
                lastRank = null;
                firstCard = null;
                cards5.Clear();
                foreach (Card c in CardArray.Where(x => x.SuitByte == flushSuit).OrderByDescending(x => x.Rank))
                {
                    if (c.RankByte == 12)
                        haveAce = true;
                    //Debug.WriteLine(c.Rank + " " + c.Suit);
                    if (lastRank == null)
                    {
                        firstCard = c;
                        cards5.Add(c);
                    }
                    else
                    {
                        if (c.RankByte == lastRank - 1)
                        {
                            cards5.Add(c);
                            straightCount2++;
                            if (straightCount2 == 4)  //the first card is not in the straightCount
                            {
                                if (firstCard.RankByte == 12)
                                    haveRoyalFlush = true;
                                else
                                    haveStraightFlush = true;
                                haveFlush = false;
                                haveStraight = false;                                     
                                break;
                            }
                            else if (haveAce && straightCount2 == 3 && firstCard.RankByte == 3)
                            {
                                haveRoyalFlush = false;
                                haveStraightFlush = true;
                                haveStraight = false;
                                haveFlush = false;
                                cards5.Add(firstCard);
                                break;
                            }
                        }
                        else
                        {
                            //if (straightCount < 6)
                            //    break;
                            straightCount2 = 0; 
                            firstCard = c;
                            cards5.Clear();
                            cards5.Add(c);
                        }
                    }
                    lastRank = c.RankByte;
                }
            }
            if(haveStraight && !haveFlush && !haveStraightFlush)
            {
                cards5.Clear();
                byte sLastRank = 0;
                foreach (Card c in CardArray.Where(x => x.RankByte <= highCard).OrderByDescending(x => x.Rank))
                {
                    if(c.RankByte != sLastRank)
                        cards5.Add(c);
                    sLastRank = c.RankByte;
                    if (cards5.Count == 5)
                        break;
                }
                if (highCard == 3)
                    cards5.Add(CardArray.FirstOrDefault(x => x.RankByte == 12));
            }
            else if(haveFlush && !haveStraightFlush)
            {
                cards5.Clear();
                foreach (Card c in CardArray.Where(x => x.SuitByte == flushSuit).OrderByDescending(x => x.Rank))
                {
                    cards5.Add(c);
                    if (cards5.Count == 5)
                        break;
                }
            }
        }

        if (haveRoyalFlush)
        {
            HandRank = HandRank.strFlush;
        }
        else if (haveStraightFlush)
        {
            HandRank = HandRank.strFlush;
        }
        else if (haveQuad)
        {
            HandRank = HandRank.quad;
        }
        else if (haveBoat)
        {
            HandRank = HandRank.boat;
        }
        else if (haveFlush)
        {
            HandRank = HandRank.flush;
        }
        else if (haveStraight)
        {
            HandRank = HandRank.straight;
        }
        else if (rankCount[3] == 1)
        {
            HandRank = HandRank.trip;
            cards5.Clear();
            int pr = -1;
            for (int i = 0; i < 13; i++)
            {
                if (rankArray[i] == 3)
                {
                    if (pr < i)
                        pr = i;
                }
            }
            foreach (Card c in CardArray.Where(x => x.RankByte == pr))
            {
                cards5.Add(c);
            }
            foreach (Card c in CardArray.Where(x => x.RankByte != pr).OrderByDescending(x => x.RankByte))
            {
                cards5.Add(c);
                if (cards5.Count == 5)
                    break;
            }
        }
        else if (rankCount[2] >= 2)  // with 7 could have 3 pairs
        {
            HandRank = HandRank.twoPr;
            cards5.Clear();
            int high = -1;
            int pr = -1;
            for (byte i = 0; i < 13; i++)
            {
                if (rankArray[i] == 2)
                {
                    if(pr < i)
                        pr = i;
                }
            }
            foreach (Card c in CardArray.Where(x => x.RankByte == pr))
            {
                cards5.Add(c);
            }
            pr = -1;
            for (byte i = 0; i < 13; i++)
            {
                if (i == cards5[0].RankByte)
                    continue;
                if (rankArray[i] == 2)
                {
                    if (pr < i)
                        pr = i;
                }
                else if (high < i && rankArray[i] > 0)
                    high = i;
            }
            foreach (Card c in CardArray.Where(x => x.RankByte == pr))
            {
                cards5.Add(c);
            }
            Card last = CardArray.FirstOrDefault(x => x.RankByte == high);
            if(last == null)
                Debug.WriteLine("twopr null");
            else
                cards5.Add(CardArray.FirstOrDefault(x => x.RankByte == high));
        }
        else if (rankCount[2] == 1)
        {
            HandRank = HandRank.onePr;
            int pr = -1;
            for (byte i = 0; i < 13; i++)
            {
                if (rankArray[i] == 2)
                {
                    pr = i;
                    foreach (Card c in CardArray.Where(x => x.RankByte == i))
                        cards5.Add(c);
                }
            }
            foreach (Card c in CardArray.Where(x => x.RankByte != pr).OrderByDescending(x => x.Rank))
            {
                cards5.Add(c);
                if (cards5.Count == 5)
                    break;
            }
        }
        else
        {
            HandRank = HandRank.high;
            foreach (Card c in CardArray.OrderByDescending(x => x.Rank))
            {
                cards5.Add(c);
                if (cards5.Count == 5)
                    break;
            }
        }
        Int64 int10zero = 10000000000;
        Score = int10zero * (int)HandRank;
        foreach (Card c in cards5)
        {
            int10zero /= 100;
            if (c == null)
                Debug.WriteLine(counter);
            else
                Score += int10zero * c.RankByte;
        }
        Cards = cards5;
    }
}
\$\endgroup\$
  • 3
    \$\begingroup\$ All of the suggestions in this answer apply to this question as well. \$\endgroup\$ – Denis Apr 1 '17 at 19:46
  • 1
    \$\begingroup\$ @Denis I wrote this before and did not go back and retrofit. I will. \$\endgroup\$ – paparazzo Apr 2 '17 at 1:19
  • \$\begingroup\$ This code looks way too large for what you are trying to archieve. I'm pretty sure my evaluators were smaller for 7 card hands. I used a simple 5 card evaluation by ordering the cards, on the 'combinations' possible from the 7 card hand. \$\endgroup\$ – Viezevingertjes Apr 3 '17 at 7:58
  • 1
    \$\begingroup\$ Performance is fine, likely faster than this though. I think it's a neglectable difference one way or another anyway but readability and the option to maintain it, is more important. If you were going for the maximum performance, this is not the way to do it anyway but it will do for simple evaluation. \$\endgroup\$ – Viezevingertjes Apr 3 '17 at 10:31
  • 1
    \$\begingroup\$ @BKSpurgeon Basically. You run multiple boards against 1 or more opponents. You can run all boards as it is less than 2 million. \$\endgroup\$ – paparazzo Apr 3 '17 at 12:48
5
\$\begingroup\$

Implement IComparable<t> and avoid the need for testing type:

public int CompareTo( Card otherCard ) {  };

If you do, MSDN docs say to also implement IEquatable<T>. Have your Object.Equals override simply call this.

public bool Equals( Card otherCard ) {  }

class Hand : Object .. is superfluous. All classes derive from Object by definition.


Be kind to your readers and future you....

By convention class members are defined at the very top of a class followed by constructors.

Puts blank lines between methods.

IsRoyalFlush is more conventional than HaveRoyalFlush. A hand is a flush. A hand does not have a flush.

Don't be lazy with variable names: c. That is excusable if it was only a for loop index counter.

field and parameter names start with lower case by convention. Properties - upper case.

What is a Quick? Very confusing. Underlying types are different in Card and Hand. Card.Quick is magically the rank, suit, and hash code.

Magic numbers everywhere!


Classes are not cohesive. The number of Constructors and the nature of the parameters is a big clue.

public Hand (List<Card> cards, Int64 score, HandRank handRank)

A Card should contain/calculate it's own score and rank. The client Hand then just asks for it:

cards[2].Score;   cards[2].Rank;  // properties or methods as appropriate

You said I chose to score the hand in the constructor - then why pass it in? This constructor is depending on the client to provide valid and correct state. You can call one constructor from another. But if the hand evaluation code were not in that other constructor (and it should not be) then this constructor probably goes away altogether.

Then there's Card.Quick. This should not be injected by any outside client code. Use more fool proof constructor parameter, perhaps a CardRank. Then quick is calculated internally and should not be exposed otherwise. If Hand needs it there is GetHashCode - which is also more descriptive of what it is.

Write properties and methods so client code is basically forced, and limited, to doing the right thing. quick is perhaps the best example of the incoherent mix of exposed raw program implementation and poker domain objects and terminology. The same implementation detail is exposed in multiple contexts. This, along with those constructor parameters force the client to understand virtually the entire class' implementation; thus client code is left to figure out what, where, and when to use this mashup and the callee is fully trusting the caller to do it right.


Explicit output for debugging should not be built into the classes. Have the poker driver class do it. Implementing ToString in all classes will help. That along w/ the existing public properties should be enough.

Yeah we use the Debug class along w/ a compiler switch to enable it but that is not license to violate single responsibility.


public Hand(Card[] CardArray, int counter)

What is counter about? debugging? And why does this constructor not care about score and HandRank? How is it that two very different constructors both build valid objects?

Why pass a card array when Hand defines a List<Card>?

Use names that use poker terminology. Not CardArray, rather newHand for example.

The scoring/evaluation must be refactored out of this constructor. If you did I suppose you would not be passing score in that other constructor. My initial cut (a little DSL there) would be separate methods for each score type.


Throwing an exception in CompareTo doesn't seem right to me, but not wrong per se. I don't see a practical difference in passing null or casting resulting in null.


P.S.

Oh, I see why you use a variable c. A subliminal reference to some other OO language.

for (int c = 0; c <= 12; c++)
\$\endgroup\$
  • \$\begingroup\$ I pass an array as the methods used to do hand analysis typically use an array. I pass counter so I can trace on problem counters. But I should pull that out. I am giving the option for the caller to score if they want to. \$\endgroup\$ – paparazzo Apr 2 '17 at 1:27
  • \$\begingroup\$ A hand can can have a flush and has a straight. If so I check if it is a royal or straight flush. c is just for card. Quick is just a number for 0 to 51 that identities a card. Pass in 7 cards and Cards is the best five used to make the hand. \$\endgroup\$ – paparazzo Apr 2 '17 at 1:39
  • \$\begingroup\$ This haphazard coding approach does not scale. That is what structured programming and OO is meant to remedy. Read Code Complete by Steve McConnell. \$\endgroup\$ – radarbob Apr 6 '17 at 4:41
-1
\$\begingroup\$

Minor but looping down is faster for finding highest

int highest = 0;
for (int i = 12; i >= 0; i--)
{
    if (rankArray[i] == 3)
    {
        highest = i;
        break;
    }
}
foreach (Card c in CardArray.Where(x => x.RankByte == highest))
    cards5.Add(c);
highest = 0;
for (int i = 12; i >= 0; i--)
{
    if (i == cards5[0].RankByte)
        continue;
    if (rankArray[i] >= 2)
    {
        highest = i;
        break;
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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