Small Poker program in C

Question

I made a small Poker program with C but I believe I could've written it better.

Full source on bitbucket.org

int checkhand (struct Card_s * hand)
{
    assert (hand) ;
    //struct Card_s *suithand = sorthand(hand, true); // Unnecessary
    struct Card_s *sorted = sorthand(hand, false);

    /* check for flushes */
    {
        bool flushsuit = false;
        int suits[4] = { [0 ... 3] = 0 };
        for (int i = 0; i < MAXHAND; ++i)
        {
            switch (hand[i].nSuit)
            {
                case spades:    suits[0]++; break;
                case clubs: suits[1]++; break;
                case hearts:    suits[2]++; break;
                case diamonds:  suits[3]++; break;
            }
        }
        for ( int f = 0 ; f < 4 ; ++f )
        {
            if (suits[f] == 5) { flushsuit = true ; } /* found a flush */
        }

        if ( flushsuit )
        {
            if ( sorted[0].nType == (sorted[1].nType-1) && sorted[1].nType == (sorted[2].nType-1) && sorted[2].nType == (sorted[3].nType-1) && sorted[3].nType == (sorted[4].nType-1) )
            {
                if ( sorted[4].nType == ace ) return 10; /* found royal flush */
                else return 9; /* found straight flush */
            }
            return 6; /* it was a normal flush */
        }
    }

    /* check for 4 of a kind */
    {
        int type [MAXTYPES ] = { [0 ... 12] = 0 }, *types ;
        types = count_types( sorted, type ) ;
        for ( int f = 0 ; f < MAXTYPES ; ++f )
        {
            if ( types[f] == 4 ) { return 8 ; } /* found 4 same types in a row */
        }
    }

    /* check for full house */
    {
        bool threeKind = false, Twopair = false ;
        int type[ MAXTYPES ] = { [0 ... 12] = 0 }, *types ;

        types = count_types( sorted, type ) ;
        for ( int f = 0 ; f < MAXTYPES ; ++f )
        {
            if ( types[f] == 3 ) { threeKind = true ; } /* found three of a kind */
        }

        int pair[MAXTYPES] = { [0 ... 12] = 0 }, *pairs ;
        pairs = count_types( sorted, pair );
        for ( int f = 0 ; f < MAXTYPES ; ++f )
        {
            if ( pairs[f] == 2 ) { Twopair = true ; } /* found normal pair */
        }
        if ( threeKind && Twopair ) return 7; /* found full house */
    }

    if ( sorted[0].nType == (sorted[1].nType-1) && sorted[1].nType == (sorted[2].nType-1) && sorted[2].nType == (sorted[3].nType-1) && sorted[3].nType == (sorted[4].nType-1) )
    {
        return 5; /* found straight */
    }

    /* check for three of a kind */
    {
        int type[ MAXTYPES ] = { [0 ... 12] = 0 }, *types ;
        types = count_types( sorted, type ) ;
        for ( int f = 0 ; f < MAXTYPES ; ++f )
        {
            if ( types[f] == 3 ) { return 4 ; } /* found three of a kind */
        }
    }

    /* check for two pair */
    {
        int type[MAXTYPES] = { [0 ... 12] = 0 }, *types ;
        types = count_types( sorted, type );
        int isTwoPair = 0 ;
        for ( int f = 0 ; f < MAXTYPES ; ++f )
        {
            if (types[f] == 2) isTwoPair++;
            if (isTwoPair == 2) { return 3 ; } /* found two pair */
        }
    }

    /* check for normal pair */
    {
        int type[MAXTYPES] = { [0 ... 12] = 0 }, *types ;
        types = count_types( sorted, type ) ;
        for ( int f = 0 ; f < MAXTYPES ; ++f )
        {
            if ( types[f] == 2 ) { return 2; } /* found normal pair */
        }
    }
    return 1 ; /* player only has a high card value */
}

Answer 1

1. A key problem is that this code only returns the kind of poker hand. Given that subsequent code will need to compare hands, like which straight is better, it makes sense that this code not only rate the kind of hand, but also provide info like top card in a straight, or rank of the cards that make up 2-pairs.

2. Inconsistent use of magic numbers and macros. Why MAXTYPES and then 12.

   //int type[MAXTYPES] = { [0 ... 12] = 0 }, *types ;
   int type[MAXTYPES] = { 0 };
   

3. Recommend to avoid declaring variables of different types on the same line. Instead consider initializing as part of the declaration

   // int type[MAXTYPES] = { [0 ... 12] = 0 }, *types ;
   // types = count_types( sorted, type );
   int type[MAXTYPES] = { [0 ... 12] = 0 };
   int *types = count_types( sorted, type );
   

4. Odd to use a enum for the suits and then magic numbers for return codes

       switch (hand[i].nSuit) {
           case spades:    suits[0]++; break;
           ...
           else return 9;
   

5. Flush code can be simplified. Only need to check if the cards all have the same suit as the first. Similar code can be used to determine a straight with sorted cards

   bool flushsuit = true;
   for (int i = 1; i < MAXHAND; ++i) {
     if (hand[0].nSuit != hand[i].nSuit) {
       flushsuit = false;
       break;
     }
   }
   

6. The redundant code for check of pairs, 3-of-a-kind, etc. can also be simplified. Given their similarities, consider combining code

   #define RANK 13
   
   group[MAXHAND+1] = { 0 };
   rankcount[RANK] = { 0 };
   for (int i = 0; i < MAXHAND; ++i) {
     int rank = hand[i].nType;
     rankcount[rank]++;
     group[rankcount[rank]]++;
   }
   
   if (group[5]) return five_of_a_kind;
   if (group[4]) return four_of_a_kind;
   if (group[3] && group[2] > 1) return full_house;
   if (group[3]) return three_of_a_kind;
   if (group[2] > 1) return two_pair;
   if (group[2]) return pair
   

7. Functions that do not change pointed-to objects, use const

    // int checkhand (struct Card_s * hand);
    int checkhand (const struct Card_s * hand);
   

8. To determine if the hand is a straight, without sorting, consider a bit array:

   unsigned ranks = 0;
   for (int i = 0; i < MAXHAND; ++i) {
     int rank = hand[i].nType;
     ranks |= 1u << rank;
   }
   
   // Replicate lo_ace as hi_ace 
   ranks |= (ranks & ACE) << RANK;
   
   #define FIVE_IN_A_ROW 0x1F
   bool IsStraight = false;
   while (ranks) {
     if (ranks & FIVE_IN_A_ROW == FIVE_IN_A_ROW) {
       IsStraight = true;
       break;
     }         
     ranks >>= 1;
   }