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

First Blackjack game in C

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

Problem

This is my first real program, though it has gone through a few major revisions. Anyhow, I am sure that there is a lot I could have done better, cleaner or safer.

Can anyone see anything I really need to work on or fix?

```
#include
#include
#include
#include
#include
#include

struct cards
{
int card_value[10];
char card_name[10];
char card_suit[10];
int card_tally;
BITMAP *card_pic[10];
};

struct cards hand[2];
short decks=1;
short cards_used[14]= {0};
int player_cash = 500;

void endgame()
{
if (player_cash (decks * 4)) && (safe_guard 49)
endgame();

cards_used[z] = cards_used[z] + 1;
safe_guard=0;

/Now Assign Values and Names to the Cards/

if ((z > 1) && (z 21)
{
hand[0].card_tally = 0;
alert("Dealer Busts!", NULL, NULL, "&Ok", NULL, 'o', 'k');
}
}

void player_turn()
{
int action=1;

while (action != 2 && hand[1].card_tally 21)
alert("Player Busts!", NULL, NULL, "&Ok", NULL, 'o', 'k');

}

int main(int argc, char **argv)
{

allegro_init();
install_keyboard();
install_mouse();
set_color_depth(16);
set_gfx_mode(GFX_AUTODETECT_WINDOWED, 400,400,0,0);

argc -= optind;
argv += optind;
if (argv[0])
decks = atoi (argv[0]);

srand(time(NULL));

int x=0;
while (x != -1)
{
display_hands();
int bet = 50;
int alert_val = alert3("Please place your bet", NULL, NULL, "&50", "&100", "15&0", '5', '1', '0');
bet = alert_val * 50;
player_cash=player_cash - bet;
display_hands();
draw_card(0);
draw_card(1);
draw_card(1);
display_hands();
player_turn();
if (hand[1].card_tally hand[1].card_tally) || (hand[0].card_tally == hand[1].card_tally
|| hand[1].card_tally > 21))
{
alert("Dealer wins!", NULL, NULL, "&Ok", NULL, 'o', 'k');
}
else
{
alert("Player wins!", N

Solution

I have some comments, some of which are quite pedantic.

  • Avoid hard-coded constants (eg 10, 'c, 'd', 'A' etc) - use defines or const



  • Inconsistent spacing around = etc



-
Use of globals is undesirable unless necessary - here it is mostly not, really. And if you must use globals in a file, make them static unless they really must be shared between files. And that is rare.

-
Make local functions static wherever possible

  • Add an explicit void in empty function parameter lists



-
Add braces on single statement:

if (condition) {
    statement;
}


-
Variable names x,y,z etc are undescriptive and unhelpful. Use of very short
names is not necessarily bad, but you should only use such names where it is
obvious what they relate to. In this code it hinders understanding (mine anyway)

-
use of short rather than int is generally pointless

-
I would prefer to see the 'hand' passed around as a pointer to struct (const if possible) rather than a hand number a

-
tally is more useful if it just sums the content of the card_value array
and returns the sum without writing to card_tally. Note that check_for_ace contains a loop that is much the same as (and therefore could use) tally

In draw_card:

-
This loop should be a function. Also there is no error check

while (hand[a].card_value[x] !=  0)
    x++;


-
The subsequent loop generates a random card that hasn't already been
used up but there should be a cards_used array for each suit. I
know your deck*4 is supposed to handle that, but does it prevent the same card (4 of hearts, say) being used 4 times (with one deck)? You might perhaps use
one array of 52 entries covering all suits. I'd put the loop in a

function too.

  • a switch might be better than many if/else



In display_hands

  • there are two almost identical loops - extract them into a function



In main

  • main is too big



  • Did you chop out some argument handling in main? What you have looks odd.



  • The reset loop in main should also be extracted into a separate function



  • Variable loop in the bitmap-destroy loop is badly named



  • END_OF_MAIN - what is this? - remove it anyway.

Code Snippets

if (condition) {
    statement;
}
while (hand[a].card_value[x] !=  0)
    x++;

Context

StackExchange Code Review Q#24568, answer score: 10

Revisions (0)

No revisions yet.