IDE: Dev-C++ - 4.9.8.7 Language: C I have finished writing a game of blackjack and would like some general feedback on the code - where I can improve it etc. One of the things I would like to do is to improve the output. I would like to be able to get more control over how things are displayed on the screen but I only have experience of using standard C library functions like printf and puts etc. So any advice here would be great.

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

#define MAXCARDS 52
#define MAXHAND 5
#define MAXSTRING 20

enum SUIT {diamond, club, heart, spade};
enum TYPE {ace, two, three, four, five, six, seven, eight, nine, ten, jack, queen, king};

//card structure

struct CARD
{
  enum SUIT suit;
  enum TYPE type;
};

struct CARD deck[MAXCARDS];//global variable


//player structure

struct PLAYER
{
  struct CARD hand[MAXHAND];
  int numofcards;
  int playertotal;
  int stick;
};

//function prototypes

void flush();

//card functions

void createDeck();
void shuffleCards();
void dealCard(struct PLAYER *, int *);
int getCardValue(struct PLAYER, int);
void displayHand(struct PLAYER);
//game functions

void initialise(struct PLAYER *, struct PLAYER *, int *);
void userMenu(struct PLAYER *, int *);
void dealerAI(struct PLAYER *, int *);
void calculateHandTotal(struct PLAYER *);
void decideWinner(struct PLAYER, struct PLAYER);

int main(int argc, char *argv[])
{  
  //seed the randam number function

  srand((unsigned)time(NULL));
  int index = 0;//index into deck of cards

  char play;
  
  struct PLAYER dealer = {0};
  struct PLAYER player = {0};
  
  createDeck();//create a deck of cards

  initialise(&dealer, &player, &index);//first time initialisation

  
  do
  {
    do
    {
      if(player.stick == 0)
      {
        printf("Player Menu\n");
        userMenu(&player, &index);
        printf("players hand\n");
        displayHand(player);
      }
      if(dealer.stick == 0)
      {
        calculateHandTotal(&dealer);
        dealerAI(&dealer, &index);
        printf("dealers hand\n");
        displayHand(dealer);
      }
    }while(dealer.stick == 0 || player.stick == 0);
    
    //calculate the dealers total

    calculateHandTotal(&dealer);
    //calculate the players total 

    calculateHandTotal(&player);
 
    //decide a winner

    decideWinner(dealer, player);
    
    flush();//to get rid of any characters in the buffer left over from menu select

    
    printf("Deal again(y/n): ");
    scanf("%c", &play);
    flush();
    
    if(play == ''y'')
      initialise(&dealer, &player, &index);   
  }while(play != ''n'');
   
  system("PAUSE");	
  return 0;
}

//function definitions


//function for carraige return problem

void flush()
{
  char ch;
  while((ch = getchar()) != ''\n'');
}

//card function definitions


//create a fresh deck of cards

void createDeck()
{
  enum SUIT i;//looping variable

  enum TYPE j;//looping variable

  int num = 0;

  for(i = diamond; i <= spade; i++)
  {
    for(j = ace; j <= king; j++)
    {
      deck[num].suit = i;
      deck[num].type = j;
      num++;
    }
  }
}

//shuffle cards

void shuffleCards()
{ 
  int i;//looping variable


  for(i = 0; i < 30; i++)
  {
    int num1 = rand() % MAXCARDS;
    int num2 = rand() % MAXCARDS;
    struct CARD temp;//swapping variable

      
    temp = deck[num1];
    deck[num1] = deck[num2];
    deck[num2] = temp;
  }
}

//deal a card

void dealCard(struct PLAYER *player, int *index)
{
  if(player->numofcards == 5)
  {
    printf("Maximum hand reached. Stick has been selected\n");
    player->stick = 1;
  }
  else
  {
    player->hand[player->numofcards] = deck[*index];
 
    (player->numofcards)++;//increment the number of cards the player has

    (*index)++;//increment the index

  }
}

int getCardValue(struct PLAYER player, int index)
{
  int value;
  
  if(player.hand[index].type == ace || player.hand[index].type < ten)
    value = player.hand[index].type + 1;
  else
    value = 10;
       
  return value;   
}

//Create the hand for the player to see

void displayHand(struct PLAYER player)
{
  char *pStringSuit[MAXHAND];//array of pointers to strings to hold the suit strings

  char *pStringType[MAXHAND];//array of pointers to strings to hold the type strings


  //suit string table

  char *pSuitName[] = {"Diamonds", "Clubs", "Hearts", "Spades"};
  //type string table

  char *pTypeName[] = {"Ace", "Two", "Three", "Four", "Five", "Six", "Seven", "Eight", "Nine", "Ten", "Jack", "Queen", "King"};
  
  int i;//looping variable

  
  //allocate memory for pointers to point to

  for(i = 0; i < player.numofcards; i++)
  {
    pStringSuit[i] = (char *) malloc(MAXSTRING);
    pStringType[i] = (char *) malloc(MAXSTRING);
    if( pStringSuit[i] == NULL )
      printf("Could not allocate memory for pStringSuit\n");
    if( pStringType[i] == NULL )
      printf("Could not allocate memory for pStringType\n");
  }
  
  //create the strings using an index into the appropriate string table

  for(i = 0; i < player.numofcards; i++)
  {
                              //index to string table

    pStringSuit[i] = pSuitName[player.hand[i].suit];
    pStringType[i] = pTypeName[player.hand[i].type];
  }
  
  //display the cards to the player

  //display all the suits first

  for(i = 0; i < player.numofcards; i++)
  {
    printf("Suit:%s\t", pStringSuit[i]);
  }
  printf("\n");
  //display the type of card

  for(i = 0; i < player.numofcards; i++)
  {
    printf("Type:%s\t", pStringType[i]);
  }
  printf("\n");
  
  //free the memory

  for(i = 0; i < player.numofcards; i++)
  {
    free(pStringSuit[i]);
      printf("Memory freed for %d\n", i);
    free(pStringType[i]); 
      printf("Memory freed for %d\n", i);
  }
}

//game function definitions


//initialise the game

void initialise(struct PLAYER *dealer, struct PLAYER *player, int *index)
{
  shuffleCards();//shuffle the cards


  *index = 0;

  dealer->numofcards = 0;
  player->numofcards = 0;
 
  dealer->playertotal = 0;
  player->playertotal = 0;
  
  dealer->stick = 0;
  player->stick = 0;
  
  //deal first two cards to player and dealer  

  dealCard(player, index);
  dealCard(dealer, index);
  dealCard(player, index);
  dealCard(dealer, index);
  
  //show the players hand

  displayHand(*player);  
}

//twist or stick

void userMenu(struct PLAYER *player, int *index)
{
  int userchoice;
  int menu = 1;
  
  while(menu)
  {
    printf("1. Twist\n");
    printf("2. Stick\n");
    printf("Enter choice: ");
    scanf("%d", &userchoice);
    switch(userchoice)
    {
      case 1: dealCard(player, index);
            menu = 0;
            break;
      case 2: player->stick = 1;
            menu = 0;
            break;
      default: printf("Please select 1 or 2\n");
    }
  }
}

//dealer AI

void dealerAI(struct PLAYER *dealer, int *index)
{
  if(dealer->playertotal <=16)
  {
    printf("Dealer takes another card\n");
    dealCard(dealer, index);
  }
  else
  {
    printf("Dealer sticks\n");
    dealer->stick = 1;
  }
}

//work out the total of a players hand

void calculateHandTotal(struct PLAYER *player)
{
  int i;//looping variable

  int hasAce = 0;
  
  player->playertotal = 0;//set player total back to zero

  
  for(i = 0; i < player->numofcards; i++)
  {
    if(player->hand[i].type == ace)
      hasAce = 1;
    player->playertotal = player->playertotal + getCardValue(*player, i);
  }
  
  //if the player has an ace and the conditions are right add another 10 to the playertotal

  if(hasAce && player->playertotal <= 11)
    player->playertotal = player->playertotal + 10;
}

//find a winner

void decideWinner(struct PLAYER dealer, struct PLAYER player)
{
  //display the total for each player

  printf("Dealers total is: %d\n", dealer.playertotal);
  printf("Your total is: %d\n", player.playertotal);
 
  //check to see if dealer is bust

  if(dealer.playertotal > 21)
    printf("Dealer is bust. You win.\n");
  
  //check to see if player is bust

  if(player.playertotal > 21)
    printf("Dealer wins. You are bust.\n");
  
  //if players are not bust, work out the winner  

  if((dealer.playertotal > player.playertotal)  && dealer.playertotal <= 21)
    printf("Dealer wins.\n");
  else
  if((player.playertotal > dealer.playertotal) && player.playertotal <= 21)
    printf("You win.\n");
  else
  if(dealer.playertotal == dealer.playertotal)
    printf("Dealer wins.\n");
}
[\source]

How about them apples?

ace_lovegrove, I'm not really trying for graphics with this game. What I meant was that I am trying to improve the screen layout of text and menus etc in this game. If you compile and run the code you will probably see what I mean. I want the screen layout to be a bit cleaner and I wanted to ask if others knew how to get more control of how things are displayed on a console. Like I said I only know how to do things with the printf function and use things like the \t, \n etc to control layout of text.

Thanks for the comments bytecoder.

How about them apples?

[edited by - popcorn on May 17, 2004 8:52:41 PM]

Anyone else got any opinions. Can I also ask if the beginner section of the forums is the best place to ask for code feedback or are there better places?

How about them apples?

I wouldn''t be completely opposed to posting this in software engineering; code-review is peer-review which is a sound engineering practice.

If you want better alignment when printing the hand, pad the strings with spaces to make them the same length.

char *pSuitName[] = {                     "Diamonds",                     "Clubs   ",                     "Hearts  ",                     "Spades  "}; 

ALL_CAPS is supposed to be reserved for #define macros. C and C++ convention is lower_case for variables and structures. CamelCase is also popular in C++ becuase it''s common with object-oriented code.

MAXCARDS, MACHAND, MAXSTRING are ok, but the enum''s and the CARD struct really shouldn''t be all caps.

You can specify values in enum, so you could do this:

enum card_type {    ace = 1,    two,    three,    four,}; 

That way the enum for two would actually have the value two.

Technically those are not function prototypes though even I still call them that. They are forware declarations (prototypes didn''t have to have the parameter list).

It''s common in C to incorporate a typedef into the struct definition so you don''t have to specify struct every time.

typedef struct card_t {  card_suit suit;  card_face face_value;} card; void shuffle_cards(card*, int cards); 

stick really is a boolean, and in C it''s common to make this explicit. It''s still an int, you just declare it using BOOL (which is a Win32 #define). You could make your own typdef to be portable. I think they added a distinct bool type to C99.

You can surround system("PAUSE") with
#ifndef NDEBUG
system("pause");
#endif
That way a release build won''t pause.

It''s bad practice to dump messages from sub-routines such as dealCards. This makes it much less modulure. dealCards should return an error code that indicates either a warning (positive value), an error (negative value), or everything''s ok (zero). stdio is your current user-interface, but you may want to replace it with graphics later (and will need to visually indicate problems that you current write to the stdout for).

The hard part about blackjack is that the ace can be either 1 or 11. getCardValue doesn''t seem to handle this.

In displayHand you don''t need to reallocate any memory.

char *pSuitName[] = {"Diamonds", "Clubs", "Hearts", "Spades"};char *pStringSuit[MAXHAND] = {0};//This works just fine, you can just point to the string in pSuitNamepStringSuit[0] = pSuitName[0]; 

Initialized is spelled with a z, oh are you from England?
Must be from England, if you want to internationalize your game, you need to use a string table and replace Twist with Hit and Stick with Stand for the US

In userMenu, you can use kbhit and getch so they player doesn''t have to hit enter to take action.

In dealerAI, delaer->stick = TRUE would make more sense to me.

I don''t think the player wins if both of them bust.

Part of BlackJack is seeing the first card the dealer has before you decide to hit or stand. This is an important part of the game!

In main, you can move initialize inside the first do, and eliminate the test for ''y''/initialise at the end of the loops.

Initialise shouldn''t display the hand, it''s job is to initiallize it. You should call the display hand function in main after calling initialise.

Thanks for that Magmai Kai Holmlor, your post was very insightful. You''ve made lots of good points that I probably wouldn''t have thought about. The point about allocating memory was actually especially good since I was having some problems freeing memory occasionally.

I always thought that function prototype and forward declarations were the same thing but I guess not now.

About the issue of seeing the dealers first card - at the moment I am using the displayhand function which shows all the cards in a hand. Should I write another function specifically to show the dealers hand so that it would display something like:

suit: unknown suit: spades
type: unknown type: king

I am reluctant to do this since most of the code would be just the same as the code in the displayhand function. So I was wondering if there was a better solution.

Lots to think about, oh you guessed right I am from the UK - Scotland rather than England. I will keep in mind the software engineering section of the forums from now on.

Thanks again.

Thats a good idea. Thanks Zahlman.

How about them apples?

Have DisplayHand take a bitmask and only display the cards whos'' bits are on. Then you could use the same function for 3, 5, or 7 card stud (Poker games).

const first_card = 1;
const second_card = 2;
displayHand(dealer, first_card);
displayHand(player, first_card|second_card);

Now I want to write a Poker game