HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Small Poker program in C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pokerprogramsmall

Problem

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 ...

Solution

-
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.

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

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


-
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 );


-
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;


-
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;
  }
}


-
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  1) return full_house;
if (group[3]) return three_of_a_kind;
if (group[2] > 1) return two_pair;
if (group[2]) return pair


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

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


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

unsigned ranks = 0;
for (int i = 0; i >= 1;
}

Code Snippets

//int type[MAXTYPES] = { [0 ... 12] = 0 }, *types ;
int type[MAXTYPES] = { 0 };
// int type[MAXTYPES] = { [0 ... 12] = 0 }, *types ;
// types = count_types( sorted, type );
int type[MAXTYPES] = { [0 ... 12] = 0 };
int *types = count_types( sorted, type );
switch (hand[i].nSuit) {
        case spades:    suits[0]++; break;
        ...
        else return 9;
bool flushsuit = true;
for (int i = 1; i < MAXHAND; ++i) {
  if (hand[0].nSuit != hand[i].nSuit) {
    flushsuit = false;
    break;
  }
}
#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

Context

StackExchange Code Review Q#127800, answer score: 2

Revisions (0)

No revisions yet.