3
\$\begingroup\$

I've completed this as part of an online programming course (in which code review is supposed to be done by fellow learners, but it's been a very long time and none of them have reviewed this), and here are the specifics:

Prompt

Use a struct to define a card as an enumerated member that is its suit value and a short that is its pips value.

Write a function that randomly shuffles the deck.

Submit your work as a text file.

Then deal out 7 card hands and evaluate the probability that a hand has no pair, one pair, two pair, three of a kind, full house and 4 of a kind. This is a Monte Carlo method to get an approximation to these probabilities. Use at least 1 million randomly generated hands. (The prompt wants us to find the probability of no pair, but doesn't give us a reference value in the standard table.)

You can check against probabilities found in a standard table.

Hand Combinations Probabilities
Royal flush 4324 0.00003232
Straight flush 37260 0.00027851
Four of a kind 224848 0.00168067
Full house 3473184 0.02596102
Flush 4047644 0.03025494
Straight 6180020 0.04619382
Three of a kind 6461620 0.04829870
Two pair 31433400 0.23495536
Pair 58627800 0.43822546
Ace high or less 23294460 0.17411920
Total 133784560 1.00000000

Here's my code:

/* A program that shuffles a deck of cards, and deals 1.4 million 7 card hands, to determine the probability of certain hand types.
   By John
   November 8 2021
*/

#include <stdio.h>
#include <stdlib.h>
#include<time.h>

#define DECK_SIZE 52    //to be used to randomly select each of 52 cards
#define SUITS_PLUS_ONE 5    //to be used to generate any of the 4 suits
#define PIPS_PLUS_ONE 14    //to be used to generate any of the 13 pips
#define DECKS_NUMBER 200000 //this * 7 hands per deck = 1.4 million hands
#define HANDS_NUMBER 1400000
#define HAND_SIZE 7

typedef enum suit
{
    hearts,
    diamonds,
    spades,
    clubs
} suit;

typedef struct playing_card
{
    suit suit;
    short pip;
} card;

card * shuffles_deck(card * ptr_to_deck, card * ptr_to_shuffled)    //shuffles the deck
{     
    int element_numbers[52]; //keeps track of selected cards to avoid repetition
    int value = 53;  //use to initalize the above array with number > 52
    
    for(int m = 0; m < 52; m++)
    {
        element_numbers[m] = value; //filling the array with value not in deck
    }
    
    for(int i = 0; i < 52; i++)
    {
        int which_element = rand() % DECK_SIZE;
        int original_i = i;  // to keep track of i's value so far
        for(int k = 0; k < 52; k++)
        {
            if(element_numbers[k] == which_element)  //ie: if card drawn before
            {
                i--;
            }
        }
           
        if(original_i == i)  //if i wsn't decremented = card not drawn before 
        {   
            element_numbers[i] = which_element;
            *(ptr_to_shuffled + i) = *(ptr_to_deck + which_element); 
        }    
     }

     return ptr_to_shuffled;
}
            
card * deals_hand(card * ptr_to_hand, card * ptr_to_shuffled)   //deals each hand and organizes cards by ascending pip value
{
    static int j = 0;
    int i = 0;
    card swapper_card;
    for( ; i < 7; i++)
    {
        *(ptr_to_hand + i) = *(ptr_to_shuffled + j);
        j++;
    }
    
    for(int index = 0; index < 7; index++)
    {
        for(int k = 0; k < 6; k++)
        {
            if((ptr_to_hand + (k+1))->pip < (ptr_to_hand + k)->pip)
            {
                swapper_card = *(ptr_to_hand + (k+1));
                *(ptr_to_hand + (k+1)) = *(ptr_to_hand + k);
                *(ptr_to_hand + k) = swapper_card;
            }
        } 
        if((ptr_to_hand + 6)->pip >= (ptr_to_hand + 5)->pip && (ptr_to_hand + 5)->pip >= (ptr_to_hand+4)->pip &&
           (ptr_to_hand + 4)->pip >= (ptr_to_hand + 3)->pip && (ptr_to_hand + 3)->pip >= (ptr_to_hand + 2)->pip &&
           (ptr_to_hand + 2)->pip >= (ptr_to_hand + 1)->pip && (ptr_to_hand + 1)->pip >= (ptr_to_hand + 0)->pip)
        {
            break;
        }

    }

    if(j == 49)  // meaning if j has reached the 50th card after which there is no more complete hand to be drawn from the deck
    {
        j = 0;
    }
    return ptr_to_hand;
}

void hand_determinator( card * ptr_to_hand) //determines what each hand contains (er: no pair, pair, etc.)
{
    static int how_many_hands = 0; //when 1000005, calculate probabilities.
    
    static int no_pair = 0, one_pair = 0, three_of_kind = 0;
    static int two_pair = 0, four_of_kind = 0, full_house = 0;
    int first_type_counter = 1;
    int second_type_counter = 1;

    card *first_type = (card *) calloc(1, sizeof(card));
    card *second_type = (card *) calloc(1, sizeof(card));
    
    if(first_type == NULL || second_type == NULL)
    {
        exit(1);
    }

    how_many_hands ++; 
    if(how_many_hands < HANDS_NUMBER)
    {
        for(int i = 0; i <HAND_SIZE; i++)
        {              
            if( (second_type + 0)->pip == 0)   //since calloc will initialize the pip value with 0          
            {
                
                if(i == 0)
                {
                    *(first_type + 0) = *(ptr_to_hand + i);
                }
                else
                {
                    if( (first_type + 0)->pip == (ptr_to_hand + i)->pip)
                    {
                        first_type_counter ++;
                        first_type = realloc(first_type, first_type_counter * sizeof(card));
                        *(first_type + (first_type_counter - 1)) = *(ptr_to_hand + i);
                    }
                    else
                    {
                        if(first_type_counter == 1)
                        {
                            *(first_type + 0) = *(ptr_to_hand + i);                            
                        }
                        else if(first_type_counter > 1)
                        {
                            *(second_type + 0) = *(ptr_to_hand + i);
                        }
                    }
                }
            }
            else
            {
                
                if( (second_type + 0)->pip == (ptr_to_hand + i)->pip)
                {
                    second_type_counter ++;
                    second_type = realloc(second_type, second_type_counter * sizeof(card));
                    *(second_type + (second_type_counter - 1)) = *(ptr_to_hand + i);
                }
                else
                {
                    if(second_type_counter == 1)
                    {
                        *(second_type + 0) = *(ptr_to_hand + i);
                    }
                    else if(second_type_counter > 1)
                    {
                        continue;
                    }
                }
            }
        }
            
        if(first_type_counter == 1 && second_type_counter == 1)
        {
            no_pair ++;
        }
        else if(first_type_counter == 2 && second_type_counter == 1)
        {
            one_pair ++;
        }
        else if(first_type_counter == 2 && second_type_counter == 2)
        {
            two_pair ++;
        }
        else if(first_type_counter == 3 && second_type_counter != 2 || first_type_counter != 2 && second_type_counter == 3)
        {
            three_of_kind ++;
        }
        else if(first_type_counter == 4 || second_type_counter == 4)
        {
            four_of_kind ++;
        }
        else if(first_type_counter == 3 && second_type_counter == 2 || first_type_counter == 2 && second_type_counter == 3) 
        {
            full_house ++;
        }   
    }                    
    else
    {
        double no_pair_prob = no_pair / (double)HANDS_NUMBER;
        double one_pair_prob = one_pair / (double)HANDS_NUMBER;
        double two_pair_prob = two_pair / (double)HANDS_NUMBER;
        double three_of_kind_prob = three_of_kind / (double)HANDS_NUMBER;
        double four_of_kind_prob = four_of_kind /(double)HANDS_NUMBER;
        double full_house_prob = full_house / (double)HANDS_NUMBER;

        printf("\n\nNo pair probablity =  %lf\nOne pair probability = %lf\nTwo pair probability = %lf\n" 
               "Three of a kind probablity = %lf\nFour of a kind probability = %lf\n"
               "Full house probability = %lf\n\n", no_pair_prob, one_pair_prob,
               two_pair_prob, three_of_kind_prob, four_of_kind_prob, full_house_prob);           
    }
        
    free(first_type);
    free(second_type);
}

int main(void)
{

    srand(time(0)); //seeding rand with current time

    card deck[52];
    card shuffled_deck[52];
    card hand[7];
    card one_card;
    static int i = 0;   //to represent the 52 cards
    //card * ptr_to_deck = deck;    
    //card * ptr_to_shuffled = shuffled_deck;
    //card * ptr_to_hand = hand;

    card * ptr_to_deck;
    card * ptr_to_shuffled;
    card * ptr_to_hand;


    for( int j = 1; j <= 13; j++)   //to generate all pip values
    {
        for(int k = 1; k <= 4; k++) //to generate all suits
        {
            one_card.suit = k;
            one_card.pip = j; 
            deck[i] = one_card;
            i++;
        }
    }
     

    for(int i  = 0; i < DECKS_NUMBER; i++)  //number of decks to exceed 1.4 M hands
    {
        ptr_to_shuffled = shuffles_deck(&deck[0], &shuffled_deck[0]); 
        for(int j = 0; j < 7; j++)
        {
            ptr_to_hand = deals_hand(&hand[0], &shuffled_deck[0]); 
            hand_determinator(&hand[0]);
        }
    }

    return 0;   
   
}
\$\endgroup\$
4
\$\begingroup\$

We start off reasonably well, defining some constants:

#define DECK_SIZE 52    //to be used to randomly select each of 52 cards
#define SUITS_PLUS_ONE 5    //to be used to generate any of the 4 suits
#define PIPS_PLUS_ONE 14    //to be used to generate any of the 13 pips

I'd argue that we shouldn't have _PLUS_ONE constants, but to simply add 1 where necessary:

#define SUITS 4
#define PIPS 13
#define DECK_SIZE (SUITS * PIPS)

Unfortunately, we then mostly fail to use the constants where appropriate, instead using magic numbers throughout the code, so it's no longer simple to change the kind of deck we're using.


We have correctly examined the return value from calloc(), but remember than realloc() can also return a null pointer if it fails, and we need to be careful about that (and also not leak the memory in that case).


There's some long-winded array access, such as this:

                        *(second_type + 0) = *(ptr_to_hand + i);

More simply written using []:

                        second_type[0] = ptr_to_hand[i];

Similarly, &deck[0] can be simply deck.


These three variables don't add anything useful - although we assign to them, we never read their values:

card * ptr_to_deck;
card * ptr_to_shuffled;
card * ptr_to_hand;

I don't think we should be copying when we shuffle the deck. There's a well-known algorithm for shuffling in place, and it looks something like this:

void shuffle_deck(card *deck, size_t length)
{
    for (size_t i = 1;  i < length;  ++i) {
        int j = rand() % (i+1); /* ignore a slight bias here */
        /* swap cards i and j */
        card tmp = deck[i];
        deck[i] = deck[j];
        deck[j] = tmp;
    }
}

Sorting the hand is inefficient, using Bubble Sort. It's only for a small number of values, so unlikely to be a major problem, but the standard library qsort() is generally a better choice (not least because it's then very obvious that we are indeed sorting).

If we adapt the program in future to count straights (and straight flushes), then we'll need to remember to allow for hands that also have pairs within the straight (e.g. 4♣ 5♣ 6♣ 6♦ 6♠ 7♣ 8♣).

It might be better not to sort, but instead to count how many of each card are present in a histogram of size PIPS.


The static variables in the hand evaluator are inconvenient. It means that the function has two responsibilities: measuring the hand's worth and printing the summary results. It actually has a third, as it decides which of those to do. It's better to have it update a suitable structure of counts:

struct hand_stats
{
    unsigned long total;
    unsigned long no_pair;
    unsigned long one_pair;
    unsigned long three_of_kind;
    unsigned long two_pair;
    unsigned long four_of_kind;
    unsigned long full_house;
};

Modified code

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define SUITS 4
#define PIPS 13
#define DECK_SIZE (SUITS * PIPS)

#define DECKS_NUMBER 200000
#define HAND_SIZE 7

typedef enum suit
{
    hearts,
    diamonds,
    spades,
    clubs
} suit;

typedef struct playing_card
{
    suit suit;
    short pip;
} card;

struct hand_stats
{
    unsigned long total;
    unsigned long no_pair;
    unsigned long one_pair;
    unsigned long three_of_kind;
    unsigned long two_pair;
    unsigned long four_of_kind;
    unsigned long full_house;
};

void shuffle_deck(card *deck, int length)
{
    for (int i = 1;  i < length;  ++i) {
        int j = rand() % (i+1);
        /* swap cards i and j */
        card tmp = deck[i];
        deck[i] = deck[j];
        deck[j] = tmp;
    }
}

void evaluate_hand(card* hand, int length, struct hand_stats *stats)
{
    unsigned value_count[PIPS] = { 0 }; /* how many cards of each pip value */
    for (int i = 0;  i < length;  ++i) {
        ++value_count[hand[i].pip];
    }

    unsigned count[SUITS+1] = { 0 }; /* how many singletons, pairs etc */
    for (int i = 0;  i < PIPS;  ++i) {
        ++count[value_count[i]];
    }

    if (count[4]) {
        ++stats->four_of_kind;
    } else if (count[3] && count[2] || count[3] >= 2) {
        ++stats->full_house;
    } else if (count[3]) {
        ++stats->three_of_kind;
    } else if (count[2] >= 2) {
        ++stats->two_pair;
    } else if (count[2]) {
        ++stats->one_pair;
    } else {
        ++stats->no_pair;
    }
    ++stats->total;
}


int main(void)
{
    srand((unsigned)time(0));

    card deck[DECK_SIZE];

    int i = 0;
    for (short j = 1;  j <= PIPS;  ++j) {
        for (short k = 1;  k <= SUITS;  ++k) {
            deck[i].suit = k;
            deck[i].pip = j;
            ++i;
        }
    }

    struct hand_stats stats = { 0, 0, 0, 0, 0, 0, 0 };
    for (int i = 0;  i < DECKS_NUMBER;  ++i) {
        shuffle_deck(deck, DECK_SIZE);
        for (int j = 0;  j + HAND_SIZE < DECK_SIZE;  j += HAND_SIZE) {
            evaluate_hand(deck+j, HAND_SIZE, &stats);
        }
    }

    /* now print the results */
    const long double total = stats.total;
    printf("No pair probablity =  %Lf\n",        stats.no_pair       / total);
    printf("One pair probability = %Lf\n",       stats.one_pair      / total);
    printf("Two pair probability = %Lf\n",       stats.two_pair      / total);
    printf("Three of a kind probablity = %Lf\n", stats.three_of_kind / total);
    printf("Four of a kind probability = %Lf\n", stats.four_of_kind  / total);
    printf("Full house probability = %Lf\n",     stats.full_house    / total);
}

One final note - we'll never get the exact results that are in a standard table, because we're counting some hands that in real poker would be evaluated higher, as a straight or flush.

\$\endgroup\$
7
  • \$\begingroup\$ if (count[3]) and if (count[2] >= 2) has an asymmetry to it. if (count[3] >= 1) and if (count[2] >= 2) or if (count[3] > 0) and if (count[2] > 1) would be more symmetric. ` \$\endgroup\$ Nov 15 '21 at 12:35
  • \$\begingroup\$ The re-use of i in for (int i = 0; i < PIPS; ++i) { ++count[value_count[i]]; } as compared to the prior for (int i = 0; i < length; ++i) looks wrong as i is indexing different units. Perhaps different index names? \$\endgroup\$ Nov 15 '21 at 12:37
  • \$\begingroup\$ With ++stats->four_of_kind, the ++ looks attacked to stats and not four_of_kind. Pre ++ vs. post ++ is a style issue, yet I'd go for stats->four_of_kind++ here. \$\endgroup\$ Nov 15 '21 at 12:42
  • \$\begingroup\$ Possibly, but the scope is very small, and i is well-understood to be a small-scope indexer, so different names might not be much clearer. I guess that's very much a matter of style, so look at your environment's other code for guidance. \$\endgroup\$ Nov 15 '21 at 12:42
  • \$\begingroup\$ 1) double is certainly sufficient vs. long double here. 2) As a matter of () usage in a #define DECK_SIZE (SUITS * PIPS) could be #define DECK_SIZE ((SUITS) * (PIPS)) to avoid dependency of other defines being simple. (Overall good review) \$\endgroup\$ Nov 15 '21 at 12:46

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.