Poker Game: Unit testing different poker hands without violating DRY principles

Question

I'm currently in the process of refactoring an old poker game which violates many of the SOLID principles.

I was trying to use the TDD approach to refactoring and I found myself having to hard code a player's hand every time I wrote a unit test for a specific poker hand check.

For example, here are the unit tests for a one pair poker hand check:

    [Test]
    public void OnePair_IsLowPair_ReturnsTrue()
    {
        SuperCard[] testHand =
        {
            new CardClub(Rank.Ace),
            new CardSpade(Rank.Ace),
            new CardDiamond(Rank.Five),
            new CardHeart(Rank.Jack),
            new CardClub(Rank.Three)
        };

        bool result = PokerHandEvaluator.OnePair(testHand);

        Assert.That(result == true);

    }

    [Test]
    public void OnePair_IsLowerMiddlePair_ReturnsTrue()
    {
        SuperCard[] testHand =
        {
            new CardClub(Rank.Eight),
            new CardSpade(Rank.Ace),
            new CardDiamond(Rank.Ace),
            new CardHeart(Rank.Jack),
            new CardClub(Rank.Three)
        };

        bool result = PokerHandEvaluator.OnePair(testHand);

        Assert.That(result == true);

    }

    [Test]
    public void OnePair_IsHigherMiddlePair_ReturnsTrue()
    {
        SuperCard[] testHand =
        {
            new CardClub(Rank.Jack),
            new CardSpade(Rank.Queen),
            new CardDiamond(Rank.Ace),
            new CardHeart(Rank.Ace),
            new CardClub(Rank.Three)
        };

        bool result = PokerHandEvaluator.OnePair(testHand);

        Assert.That(result == true);

    }

    [Test]
    public void OnePair_IsHighPair_ReturnsTrue()
    {
        SuperCard[] testHand =
        {
            new CardClub(Rank.Jack),
            new CardSpade(Rank.Deuce),
            new CardDiamond(Rank.Six),
            new CardHeart(Rank.Ace),
            new CardClub(Rank.Ace)
        };

        bool result = PokerHandEvaluator.OnePair(testHand);

        Assert.That(result == true);

    }

This is what the class diagram (at the project's current stage) looks like:

My question is how I should go about extracting a method (or methods) out of the testHand initialization.

Answer 1

One thing that would help with reducing duplication is to use parameterized tests.

Additionally, you might want to have a enum with the different hand types instead of - or in addition to - having one method for each hand type, such as your current PokerHandEvaluator.OnePair.

Answer 2

IMO I'd join all of your tests into one. And just rotate through them.

private static void Rotate<T>(List<T> list) {
    T first = list[0];
    list.RemoveAt(0);
    list.Add(first);
}

[Test]
public void TestOnePairs()
{
    List<SuperCard> testHand = new List<SuperCard> {
        new CardClub(Rank.Ace),
        new CardSpade(Rank.Ace),
        new CardDiamond(Rank.Two),
        new CardHeart(Rank.Three),
        new CardClub(Rank.Four)
    };

    for (let i=0; i < 5; i++) {
        Assert.That(PokerHandEvaluator.OnePair(testHand.ToArray()));
        Rotate(testHand);
    }
}

If this were Python I'd suggest changing List<SuperCard> to List<Tuple<Suit, Rank>>. However IIRC C# doesn't like Tuples too much. And so you may want to make them List<Card>, but pass both of the enums to the card at instantiation.

---

I should note that this doesn't test all combinations that the pairs can be. And so you should really use something like Pythons itertools.combinations.

This is as the following pseudocode of OnePair would be incorrect, but would pass your tests.

def OnePair(values):
    prev = values[-1]
    for curr in values:
        if curr.rank == prev.rank:
            return True
        prev = curr
    return False

Answer 3

I decided to post a self-answer as suggested by t3chbot because the following code example is just an attempt at following Simon's suggestion; I haven't fully incorporated the code as I have not managed to resolve my original question. I hope this is okay.

public class OnePairTestDataSource : IEnumerable
    {
        public IEnumerator GetEnumerator()
        {
            yield return new SuperCard[]
            {
                new CardClub(Rank.Ace),
                new CardSpade(Rank.Ace),
                new CardDiamond(Rank.Five),
                new CardHeart(Rank.Jack),
                new CardClub(Rank.Three)
            };

            ...
        }

    [TestCaseSource(typeof(OnePairTestDataSource))]
        public void OnePair_IsOnePair_ReturnsTrue(SuperCard[] pTestHand)
        {
            bool result = PokerHandEvaluator.OnePair(pTestHand);
            Assert.That(result == true);
        }

Even though I like Simon's suggestion of using parameterized tests, I noticed that I still have to do some repetition in order to initialize an array that I want to test. Is this what you had in mind Simon? If not could you clarify with a code example perhaps?

---

I tried using a parameterized test source as well. Here is another implementation which I like better than the one above and my original one, but still leaves me with the same problem:

static IEnumerable<SuperCard[]> GetTestHand(PokerHand pPokerHand)
{
   if (pPokerHand == PokerHand.OnePair)
   {
     yield return new SuperCard[]
     {
       new CardClub(Rank.Ace),
       new CardSpade(Rank.Ace),
       new CardDiamond(Rank.Five),
       new CardHeart(Rank.Jack),
       new CardClub(Rank.Three)
     };

     ...
   }
}

[TestCaseSource(nameof(GetTestHand), new object[] { PokerHand.OnePair })]
public void OnePair_IsOnePair_ReturnsTrue(SuperCard[] pTestHand)
{
   bool result = PokerHandEvaluator.OnePair(pTestHand);
   Assert.That(result == true);
}

Answer 4

Not sure if there is any advantage of having 4 classes (CardClub, CardSpade, CardHeart, CardDiamond) with one property Rank,

compared to

a Card class with 2 properties, Suit (of Type Suit enum) and Rank (of Type Rank enum).

Have a CardFactory class, that has a static method

Card CreateCard(Suit suit, Rank rank) => new Card(suit, rank);

We could have a HandTestDataGenerator class with a static method for generating test hands:

public static IList<Card> CreateCards(Dictionary<Suit, List<Rank> cards) =>
    cards.SelectMany(x => x.Value.Select(rank => new Card(x.Key, rank));

Now for each test data, create a static method in HandTestDataGenerator like:

IList<Card> GenerateLowPairHand()
{
    var lowPairHand = new Dictionary<Suit, List<Rank>>();
    lowPairHand.Add(Suit.Club, new List<Rank>{ Rank.Ace, Rank.Three });
    //Add other suits and ranks here
    return CreateCards(lowPairHand);
}

From TestMethod, call

SuperCard[] testHand = HandTestDataGenerator.GenerateLowPairHand();