Defining what combination the user have in Poker

Question

I'm developing a C# Poker app and I currently use the modulus and the divide operator % , / and dividing by 4 because there are 4 cards of the same type but with different colors. The deck of cards for playing Poker consists of 52 cards. I start counting them from 0. If we take the cards starting from Ace up to King, we will have 0-51. What we can do from here is assume that each Ace (0,1,2,3) has result when divided by 4, 0 (0 /4 = 0, 1 / 4 = 0) so on. With this, I get the current type of the card.

To determine the suit/color of the card, I am using the % operator because the clubs are 0-4-8-12-16. When we divide 0,4,8,12,16 by 4 (4 % 4) we get result 0. These are the Clubs and Diamonds: 1-5-9-13-17 and 1 % 4 = 1.

I don't have any class for the card itself, so is there any better alternative to this?

Here are some of the methods I use to find specific combinations:

public class Hand
{
    public enum Combinations
    {
        Default = -1,
        HighCard = -1,
        PairTable = 0,
        Pair = 1,
        TwoPair = 2,
        ThreeOfAKind = 3,
        Straight = 4,
        Flush = 5,
        FullHouse = 6,
        FourOfAKind = 7,
        StraightFlush = 8,
        RoyalFlush = 9            
    }
    public double Power { get; set; }
    public int Current { get; set; }
}

straight[] contains the player's 2 cards and the five cards on the table.

    private UsersProperties RFourOfAKind(UsersProperties user, IReadOnlyList<int> straight)
    {
        const int localType = (int) Hand.Combinations.FourOfAKind;
        for (int j = 0; j <= 3; j++)
        {
            if (straight[j]/4 == straight[j + 1]/4 && straight[j]/4 == straight[j + 2]/4 &&
                straight[j]/4 == straight[j + 3]/4)
            {
                UpdateHand(user, localType, straight[j]/4*4);
            }
            if (straight[j]/4 == (int) Cards.Numbers.Ace && straight[j + 1]/4 == (int) Cards.Numbers.Ace &&
                straight[j + 2]/4 == (int) Cards.Numbers.Ace && straight[j + 3]/4 == (int) Cards.Numbers.Ace)
            {
                UpdateHand(user, localType, 13*4);
            }
        }
        return user;
    }

FullHouse method:

 private UsersProperties RFullHouse(UsersProperties user, int[] straight)
        {
            const int localType = (int) Hand.Combinations.FullHouse;
            for (int j = (int) Cards.Numbers.Ace; j <= (int) Cards.Numbers.King*4; j++)
            {
                int two = -1;
                int counterForThree = straight.Count(rep => rep/4 == j/4);
                if (counterForThree == 3)
                {
                    for (int k = (int) Cards.Numbers.Ace; k <= (int) Cards.Numbers.King*4; k++)
                    {
                        if (k/4 == j/4) continue;
                        int counterForTwo = straight.Count(rep => rep/4 == k/4);
                        if (counterForTwo != 2 || k/4 == two) continue;
                        two = k/4;
                        if (j == 0)
                        {
                            UpdateHand(user, localType, 13*3 + (k/4)*2);
                        }
                        else if (k == 0)
                        {
                            UpdateHand(user, localType, j/4*3 + 13*2);
                        }
                    }
                }
            }
            return user;
        }

Straight method:

        private UsersProperties RStraight(UsersProperties user, IEnumerable<int> straight)
    {
        const int localType = (int) Hand.Combinations.Straight;
        int[] op = straight.Select(o => o/4).Distinct().ToArray();
        for (int j = 0; j < op.Length - 4; j++)
        {
            if (op[j] + 1 == op[j + 1] && op[j] + 2 == op[j + 2] && op[j] + 3 == op[j + 3] && op[j] + 4 == op[j + 4])
            {
                UpdateHand(user, localType, op.Max() - 4 == op[j] ? op.Max() : op[j + 4]);
            }
            if (op[j] != (int) Cards.Numbers.Ace || op[j + 1] != (int) Cards.Numbers.Ten ||
                op[j + 2] != (int) Cards.Numbers.Jack || op[j + 3] != (int) Cards.Numbers.Queen ||
                op[j + 4] != (int) Cards.Numbers.King) continue;

            UpdateHand(user, localType,13);
        }
        return user;
     }

UpdateHand method:

private void UpdateHand(UsersProperties user, int combinationType, int powerType)
    {
        user.Type = combinationType;
        user.Power = powerType + user.Type * 100;
        _win.Add(new Hand { Power = user.Power, Current = user.Type });
    }

Answer 1

It seems like you could make the code much more readable if you just used enums for the values and suits...

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

public enum SuitType
{
    Heart,
    Diamond,
    Club,
    Spade
}

public class Card
{
    public CardType Value { get; set; }
    public SuitType Suit { get; set; }
}

Then the code for the calculations would be a lot easier to read...

public interface ICombinationAnalyzer
{
    bool Check(IReadOnlyList<Card> hand);
}

public class RoyalFlush : ICombinationAnalyzer
{
    public bool Check(IReadOnlyList<Card> hand)
    {
        bool flush = hand.GroupBy(c => c.Suit).Count() == 1;

        bool royal = false;
        if (flush)
        {
            var sortedHand = hand.OrderBy(c => c.Value).ToArray();
            royal = sortedHand[0].Value == CardType.Ace && sortedHand[1].Value == CardType.King && sortedHand[2].Value == CardType.Queen && sortedHand[3].Value == CardType.Jack && sortedHand[4].Value == CardType.Ten;
        }

        return flush && royal;
    }
}

public class FourofAKind : ICombinationAnalyzer
{
    public bool Check(IReadOnlyList<Card> hand)
    {
        return hand.GroupBy(c => c).Any(g => g.Count() >= 4);
    }
}

public class FullHouse : ICombinationAnalyzer
{
    public bool Check(IReadOnlyList<Card> hand)
    {
        // If there are only two different values in hand and exactly three of those cards are of one value (no need to check that the other two cards are the other value)
        return hand.GroupBy(c => c.Value).Count() == 2 && hand.GroupBy(c => c.Value).Any(g => g.Count() == 3);
    }
}

etc...

Notice Combination Analyzer code implements an interface and each type of combination is a different class. This segregates the code for each combination into a separate class, making them easier to unit test and maintain.

This way you can use the Strategy Pattern to iterate through the entire set of combinations, starting with the highest value one first and stopping as soon as one of the methods returns true.

public class AnalyzeCombinations
{
    private readonly ICombinationAnalyzer[] _combinations;

    // Pass in the list of analyzers with the highest value one first
    public AnalyzeCombinations(params ICombinationAnalyzer[] combinations)
    {
        _combinations = combinations;
    }

    public ICombinationAnalyzer Check(IReadOnlyList<Card> hand)
    {
        return _combinations.FirstOrDefault(combination => combination.Check(hand));
    }
}

There are several improvement you could make to my example as well, such as creating a separate method for a flush or a straight and then calling that in each of the combination analyzers that needs those tests (such as Straight, Flush, Royal Flush, etc.).

Of course, this is just one of numerous ways to write your code, hopefully it will give you an idea on how to use patterns such as the Strategy Pattern, Separation of Concerns, Single Responsibility Principal and other design patterns.

EDIT: I didn't include the logic to determine the winning hand but I would recommend doing that in another class or set of classes, not adding it to the classes in the template. This is because the current classes in the template only know about one hand. And Separation of Concerns dictates another set of classes will be needed to hand to handle the collection of hands.

To determine the winning hand I would recommend starting by iterating through the hands and calling GetRank. The winning hand will be the one with the highest rank. if there is a tie then you will need additional logic. For example, if there are two full houses then you will need to determine which hones has the highest Card Type. Possibly a good implementation would be to add a GetPower method to each of the analyzers (passing in the hand) so you can calculate the "power" of the hand. It is likely the logic you are using to calculate the power will already work. You just have to put the logic in each analyzer. Then if GetRank ends in a tie the you can call GetPower to see which one wins.

hmmmm...now that I am writing this I think that would be another fun thing to figure out. Maybe I will have time to work on that this week. I will also fix the bug to support an A-2-3-4-5 straight.

If you want to make improvements to the github solution you are welcome to contribute.