8
\$\begingroup\$

I made a Poker game a while ago and looking at it now, I'm not really happy with the code structure and I decided to revamp some of the core functionalities there, starting with the different Poker combinations.

Card.cs

Obviously the first and most important thing are the cards themselves:

public enum CardColor
{
    Hearth,
    Club,
    Spade,
    Diamond
}

public enum CardSuit
{
    Two,
    Three,
    Four,
    Five,
    Six,
    Seven,
    Eigth,
    Nine,
    Ten,
    Jack,
    Queen,
    King,
    Ace
}

public class Card : IComparable<Card>
{
    public CardColor CardColor { get; }
    public CardSuit CardSuit { get; }

    public Card(CardColor cardColor, CardSuit cardSuit)
    {
        CardColor = cardColor;
        CardSuit = cardSuit;
    }

    public int CompareTo(Card other) => CardSuit.CompareTo(other.CardSuit);
}

It's pretty simple immutable object which allows comparison between one another.

Hand.cs

Next we have the Hand class which contains an array of 2 cards and an instance of the HandRank class which contains all neccesary info in order to determine which hand is strong (hand power, kicker, etc.):

public enum HandPower
{
    HighCard,
    Pair,
    TwoPair,
    ThreeOfAKind,
    Straight,
    Flush,
    FullHouse,
    FourOfAKind,
    StraightFlush,
    RoyalFlush,
}

public class Hand : IComparable<Hand>
{
    public Card[] Cards { get; }
    public HandRank HandRank { get; private set; }

    public Hand(Card[] cards)
    {
        if (cards == null || cards.Length != 2)
        {
            throw new ArgumentException(nameof(cards));
        }
        Cards = cards;
    }

    public void UpdateHandRank(HandRank newHandRank) => HandRank = newHandRank;

    public int CompareTo(Hand other)
    {
        if (ReferenceEquals(this, other))
        {
            return 0;
        }
        if (ReferenceEquals(null, other))
        {
            return 1;
        }
        return HandRank.CompareTo(other.HandRank);
    }
}

HandRank.cs

public class HandRank : IComparable<HandRank>
{
    public HandPower HandPower { get; }
    public CardSuit CardSuit { get; }
    public Card Kicker { get; }

    public HandRank(HandPower handPower, CardSuit cardSuit, Card kicker)
    {
        HandPower = handPower;
        Kicker = kicker;
        CardSuit = cardSuit;
    }

    public int CompareTo(HandRank other)
    {
        if (ReferenceEquals(this, other))
        {
            return 0;
        }
        if (ReferenceEquals(null, other))
        {
            return 1;
        }
        if (HandPower == other.HandPower)
        {
            if (CardSuit == other.CardSuit)
            {
                if (Kicker == null || other.Kicker == null)
                {
                    return Convert.ToInt32(Kicker != null || other.Kicker == null)
                        .CompareTo(Convert.ToInt32(Kicker == null || other.Kicker != null));
                }
                return Kicker.CompareTo(other.Kicker);
            }
            return CardSuit.CompareTo(other.CardSuit);
        }
        return HandPower.CompareTo(other.HandPower);
    }
}

ICombinationAnalyzer.cs

This is the base interface that all the combinations inherit, it contains a single method HandRank AnalyzeCombination();:

public interface ICombinationAnalyzer
{
    HandRank AnalyzeCombination();
}

This is later inherited by the CombinationAnalyzer abstract class:

CombinationAnalyzer.cs

public abstract class CombinationAnalyzer : ICombinationAnalyzer
{
    protected Card[] playerCards;
    protected Card[] tableCards;
    protected IEnumerable<Card> allCards;

    protected CombinationAnalyzer(Card[] playerCards, Card[] tableCards)
    {
        if (playerCards == null || playerCards.Length != 2)
        {
            throw new ArgumentException(nameof(playerCards));
        }
        if (tableCards == null || tableCards.Length > 5)
        {
            throw new ArgumentException(nameof(tableCards));
        }
        this.playerCards = playerCards;
        this.tableCards = tableCards;
        allCards = playerCards.Concat(tableCards);
    }

    protected Card FindKicker(IEnumerable<Card> cardsSource, Func<Card, bool> predicate)
    {
        Card[] playerMatchingCards = playerCards.Where(predicate).ToArray();
        if (playerMatchingCards.Length == playerCards.Length)
        {
            return playerMatchingCards.FindMaxCardBySuit();
        }
        return playerMatchingCards.Length == 1 ? playerMatchingCards[0] : null;
    }

    protected Card FindKicker(IEnumerable<CardSuit> cardsSource, Func<CardSuit, bool> predicate)
    {
        Card[] playerMatchingCards = playerCards.Where(card => predicate(card.CardSuit)).ToArray();
        if (playerMatchingCards.Length == playerCards.Length)
        {
            return playerMatchingCards.FindMaxCardBySuit();
        }
        return playerMatchingCards.Length == 1 ? playerMatchingCards[0] : null;
    }

    public abstract HandRank AnalyzeCombination();
}

This is the actual class that all the combinations inherit directly. It contains all the necessary members to reduce the code in the derived class and ease their life. This class also makes use of my only extension method:

public static class Extensions
{
    public static Card FindMaxCardBySuit(this IEnumerable<Card> source)
    {
        Card maxCard = null;
        CardSuit maxSuit = CardSuit.Two;
        foreach (var playerCard in source)
        {
            if (playerCard.CardSuit > maxSuit)
            {
                maxSuit = playerCard.CardSuit;
                maxCard = playerCard;
            }
        }
        return maxCard;
    }
}

Here are all the combination analyzer classes:

PairAnalyzer.cs

public class PairAnalyzer : CombinationAnalyzer
{
    public PairAnalyzer(Card[] playerCards, Card[] tableCards) : base(playerCards, tableCards)
    {

    }

    public override HandRank AnalyzeCombination()
    {
        var pairGroups = allCards.GroupBy(card => card.CardSuit).Where(group => group.Count() == 2).ToArray();
        if (!pairGroups.Any())
        {
            return new HandRank(default(HandPower), default(CardSuit), null);
        }
        IEnumerable<Card> combinationCards = pairGroups.SelectMany(group => group);
        CardSuit maxSuit = combinationCards.Max(card => card.CardSuit);
        return new HandRank(HandPower.Pair, maxSuit, FindKicker(combinationCards, card => card.CardSuit != maxSuit));
    }
}

TwoPairAnalyzer.cs

public class TwoPairAnalyzer : CombinationAnalyzer
{
    public TwoPairAnalyzer(Card[] playerCards, Card[] tableCards) : base(playerCards, tableCards)
    {
    }

    public override HandRank AnalyzeCombination()
    {
        var pairGroups = allCards.GroupBy(card => card.CardSuit).Where(group => group.Count() == 2).ToArray();
        if (pairGroups.Length <= 1)
        {
            return new HandRank(default(HandPower), default(CardSuit), null);
        }
        IEnumerable<Card> combinationCards = pairGroups.SelectMany(group => group);
        CardSuit maxSuit = combinationCards.Max(card => card.CardSuit);
        Func<Card, bool> predicate =
            card => combinationCards.All(combination => combination.CardSuit != card.CardSuit);
        if (pairGroups.Length > 2)
        {
            CardSuit minSuit = combinationCards.Min(card => card.CardSuit);
            combinationCards = combinationCards.Where(
                combination => combination.CardSuit != minSuit);
        }
        return new HandRank(HandPower.TwoPair, maxSuit, FindKicker(combinationCards, predicate));
    }
}

ThreeOfAKindAnalyzer.cs

public class ThreeOfAKindAnalyzer : CombinationAnalyzer
{
    public ThreeOfAKindAnalyzer(Card[] playerCards, Card[] tableCards) : base(playerCards, tableCards)
    {
    }

    public override HandRank AnalyzeCombination()
    {
        var pairGroups = allCards.GroupBy(card => card.CardSuit).Where(group => group.Count() == 3).ToArray();
        if (pairGroups.Length < 1)
        {
            return new HandRank(default(HandPower), default(CardSuit), null);
        }
        IEnumerable<Card> combinationCards = pairGroups.SelectMany(group => group);
        CardSuit maxSuit = combinationCards.Max(card => card.CardSuit);
        return new HandRank(HandPower.ThreeOfAKind, maxSuit,
            FindKicker(combinationCards, card => card.CardSuit != maxSuit));
    }
}

StraightAnalyzer.cs

public class StraightAnalyzer : CombinationAnalyzer
{
    public StraightAnalyzer(Card[] playerCards, Card[] tableCards) : base(playerCards, tableCards)
    {
    }

    public override HandRank AnalyzeCombination()
    {
        var orderedCards = allCards.OrderBy(card => card.CardSuit).ToList();
        Func<Card, bool> predicate =
            card => tableCards.Any(orderedCard => orderedCard.CardSuit == card.CardSuit) ||
                    orderedCards.All(orderedCard => orderedCard.CardSuit != card.CardSuit);
        for (int i = 0; i < orderedCards.Count - 1; i++)
        {
            if (orderedCards[i].CardSuit + 1 != orderedCards[i + 1].CardSuit &&
                (orderedCards[i].CardSuit != CardSuit.Five || orderedCards[i + 1].CardSuit != CardSuit.Ace))
            {
                orderedCards.Remove(orderedCards[i + 1]);
                i--;
            }
        }
        if (orderedCards.Count < 5)
        {
            return new HandRank(default(HandPower), default(CardSuit), null);
        }

        if (orderedCards[0].CardSuit == CardSuit.Two && orderedCards[1].CardSuit == CardSuit.Three &&
            orderedCards[2].CardSuit == CardSuit.Four && orderedCards[3].CardSuit == CardSuit.Five &&
            orderedCards[4].CardSuit == CardSuit.Ace)
        {
            return new HandRank(HandPower.Straight, CardSuit.Five, FindKicker(orderedCards, predicate));
        }
        return new HandRank(HandPower.Straight, orderedCards.Max(card => card.CardSuit),
            FindKicker(orderedCards, predicate));
    }
}

FlushAnalyzer.cs

public class FlushAnalyzer : CombinationAnalyzer
{
    public FlushAnalyzer(Card[] playerCards, Card[] tableCards) : base(playerCards, tableCards)
    {
    }

    public override HandRank AnalyzeCombination()
    {
        var groupedCards = allCards.GroupBy(card => card.CardColor).SingleOrDefault(group => group.Count() >= 5);
        if (groupedCards != null)
        {
            IEnumerable<Card> combinationCards = groupedCards.Select(group => group);
            return new HandRank(HandPower.Flush, groupedCards.Max(card => card.CardSuit),
                FindKicker(combinationCards,
                    card => groupedCards.Count() > 5 &&
                            card.CardSuit != combinationCards.Max(combinationCard => combinationCard.CardSuit)));
        }
        return new HandRank(default(HandPower), default(CardSuit), null);
    }
}

FullHouseAnalyzer.cs

public class FullHouseAnalyzer: CombinationAnalyzer
{
    public FullHouseAnalyzer(Card[] playerCards, Card[] tableCards) : base(playerCards, tableCards)
    {
    }

    public override HandRank AnalyzeCombination()
    {
        var pairGroups = allCards.GroupBy(card => card.CardSuit).Where(group => group.Count() == 2).ToArray();
        var tripletGroups = allCards.GroupBy(card => card.CardSuit).Where(group => group.Count() == 3).ToList();
        IEnumerable<Card> combinationCards =
            pairGroups.SelectMany(group => group).Concat(tripletGroups.SelectMany(group => group));
        HandRank bestTriplet = new ThreeOfAKindAnalyzer(playerCards, tableCards).AnalyzeCombination();
        if (tripletGroups.Count == 2)
        {
            var lowerTriplet = tripletGroups.FirstOrDefault(group => group.Key == bestTriplet.CardSuit);
            return new HandRank(HandPower.FullHouse, bestTriplet.CardSuit,
                FindKicker(combinationCards,
                    card => card.CardSuit != lowerTriplet.Key && card.CardSuit != bestTriplet.CardSuit));
        }
        if (tripletGroups.Count == 1 && pairGroups.Length >= 1)
        {
            HandRank bestPair = new PairAnalyzer(playerCards, tableCards).AnalyzeCombination();
            return new HandRank(HandPower.FullHouse, bestTriplet.CardSuit,
                FindKicker(combinationCards,
                    card => card.CardSuit != bestPair.CardSuit && card.CardSuit != bestTriplet.CardSuit));
        }
        return new HandRank(default(HandPower), default(CardSuit), null);
    }
}

FourOfAKindAnalyzer.cs

public class FourOfAKind : CombinationAnalyzer
{
    public FourOfAKind(Card[] playerCards, Card[] tableCards) : base(playerCards, tableCards)
    {
    }

    public override HandRank AnalyzeCombination()
    {
        var pairGroups = allCards.GroupBy(card => card.CardSuit).Where(group => group.Count() == 4).ToArray();
        if (pairGroups.Length < 1)
        {
            return new HandRank(default(HandPower), default(CardSuit), null);
        }
        IEnumerable<Card> combinationCards = pairGroups.SelectMany(group => group);
        CardSuit maxSuit = combinationCards.Max(card => card.CardSuit);
        return new HandRank(HandPower.FourOfAKind, maxSuit,
            FindKicker(combinationCards, card => card.CardSuit != maxSuit));
    }
}

StraightFlushAnalyzer.cs

public class StraightFlushAnalyzer : CombinationAnalyzer
{
    public StraightFlushAnalyzer(Card[] playerCards, Card[] tableCards) : base(playerCards, tableCards)
    {
    }

    public override HandRank AnalyzeCombination()
    {
        var straight = new StraightAnalyzer(playerCards, tableCards).AnalyzeCombination();
        var flush = new FlushAnalyzer(playerCards, tableCards).AnalyzeCombination();

        if (flush.HandPower != default(HandPower) && straight.HandPower != default(HandPower))
        {
            CardSuit lowestCard = straight.CardSuit - 5;
            IEnumerable<CardSuit> straightFlushSuits = straight.CardSuit == CardSuit.Five
                ? new[] {CardSuit.Ace, CardSuit.Two, CardSuit.Three, CardSuit.Four, CardSuit.Five}
                : Enum.GetValues(typeof(CardSuit)).Cast<CardSuit>().Skip((int) (lowestCard - 1)).Take(5);
            if (straightFlushSuits.All(suit => suit == flush.CardSuit))
            {
                return new HandRank(HandPower.StraightFlush, straight.CardSuit,
                    FindKicker(straightFlushSuits,
                        cardSuit => straightFlushSuits.All(suit => suit != cardSuit) ||
                                    tableCards.Any(card => card.CardSuit == cardSuit)));
            }
        }
        return new HandRank(default(HandPower), default(CardSuit), null);
    }
}

And lastly, I have a class which finds the best combination you can form from your hand and from the table:

CombinationChecker.cs

public class CombinationChecker
{
    private readonly IEnumerable<Type> combinationAnalyzerTypes;

    public CombinationChecker()
    {
        combinationAnalyzerTypes = GetTypesOfBaseType(typeof(CombinationAnalyzer));
    }

    public HandRank GetBestCombination(Card[] playerCards, Card[] tableCards)
        => GetCombinationAnalyzerInstances<CombinationAnalyzer>(new object[] {playerCards, tableCards})
            .Max(combination => combination.AnalyzeCombination());

    private IEnumerable<Type> GetTypesOfBaseType(Type baseType)
        => Assembly.GetExecutingAssembly().GetTypes().Where(t => baseType.IsAssignableFrom(t) && t != baseType);

    private IEnumerable<T> GetCombinationAnalyzerInstances<T>(object[] parameters)
        where T : class => combinationAnalyzerTypes.Select(type => (T) Activator.CreateInstance(type, parameters));
}
\$\endgroup\$
  • 2
    \$\begingroup\$ Not really directly code issues but poker. What you call color is really suite. 2-A is card rank. Straight-flush to high card is hand rank. The two cards in your hand are hole cards. Table is called the board. You are using kicker wrong - it is the card(s) not part of the made hand. It can take up to 5 cards (high card and flush) to determine the relative strength of two equal rank hands. No reason to differentiate a royal as it is just an ace high straight flush. Treating hand ranks independently is very inefficient compared to a single comparison. \$\endgroup\$ – paparazzo Feb 2 '17 at 22:54
  • \$\begingroup\$ @Paparazzi thank you for the notes, I was wondering if I am doing stuff properly as I'm trying to stick to the rules as much as possible, but right now I'm checking if the combination contains any of the player's cards and if that's the case this card cant be counted as a kicker if both cards are in the combination, you have no kicker, but if there is duplicate card on the table i.e on the board you have ace-2-3-4 and in your hand you have ace and 5 you have straight from ace to 5 with ace kicker. But I guess that's wrong right? \$\endgroup\$ – Denis Feb 2 '17 at 23:00
  • 1
    \$\begingroup\$ Combination is also a term not to use as it is used for for statistical analysis of poker. Wiki is a pretty good source for poker terms and hand ranks. Need to be prepared to use all five cards to compare two equal hand ranks. \$\endgroup\$ – paparazzo Feb 2 '17 at 23:05
  • \$\begingroup\$ The 5 in 5 high straight is not a kicker - it is a 5 high straight. TT885 then the 5 is a kicker. \$\endgroup\$ – paparazzo Feb 2 '17 at 23:06
  • \$\begingroup\$ @Paparazzi Cards have a "suit", not a "suite" (see dictionary.com/browse/suit?s=t definition 7) \$\endgroup\$ – Brian J Apr 26 '17 at 16:27
2
\$\begingroup\$

I don't like the required length in the ctor for Hand. Allowing the user to provide an array signals that you don't care about the number of cards provided. If there must be a specific number of cards, you should make the user provide them explicitly, and if you need to, concatenate them into an array in the constructor. If this was a public library, this would probably be quite the pain point for your users, and when it comes down to it, you are basically just a user 6 weeks/months after you write this.

public Hand(Card[] cards)
{
    if (cards == null || cards.Length != 2)
    {
        throw new ArgumentException(nameof(cards));
    }
    Cards = cards;
}

public HandRank HandRank { get; private set; }
public void UpdateHandRank(HandRank newHandRank) => HandRank = newHandRank;

Just make the property's set publicly exposed.


The amount of nesting here is horrible and kind of confusing. I would switch these ifs around:

if (HandPower == other.HandPower)
{
    if (CardSuit == other.CardSuit)
    {
        if (Kicker == null || other.Kicker == null)
        {
            return Convert.ToInt32(Kicker != null || other.Kicker == null)
                .CompareTo(Convert.ToInt32(Kicker == null || other.Kicker != null));
        }
        return Kicker.CompareTo(other.Kicker);
    }
    return CardSuit.CompareTo(other.CardSuit);
}
return HandPower.CompareTo(other.HandPower);

Something like:

if (HandPower = other.HandPower)
{
    return HandPower.CompareTo(other.HandPower);
}
if (CardSuit != other.CardSuit)
{
    return HandPower.CompareTo(other.HandPower);
}
if (Kicker == null || other.Kicker == null)
{
    return Convert.ToInt32(Kicker != null || other.Kicker == null)
        .CompareTo(Convert.ToInt32(Kicker == null || other.Kicker != null));
}
return Kicker.CompareTo(other.Kicker);

if (orderedCards[0].CardSuit == CardSuit.Two && orderedCards[1].CardSuit == CardSuit.Three &&
    orderedCards[2].CardSuit == CardSuit.Four && orderedCards[3].CardSuit == CardSuit.Five &&
    orderedCards[4].CardSuit == CardSuit.Ace)

I would put each of these conditions on its own line to emphasize the fact that they all have equal weight. Also (this may be better in your IDE than in the SE chat), they are just a little long for having two on a line.

\$\endgroup\$
  • \$\begingroup\$ Thanks for your input. I agree on your points, but how do you feel about the overall readability of the code? \$\endgroup\$ – Denis May 20 '17 at 21:06
  • 1
    \$\begingroup\$ I think it is pretty good. All those predicates make it a little difficult to read, but I liked the design, so I didn't say anything. \$\endgroup\$ – user34073 May 20 '17 at 23:20

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.