patterncModerate
First Blackjack game in C
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
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.
-
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 braces on single 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
-
I would prefer to see the 'hand' passed around as a pointer to struct (const if possible) rather than a hand number
-
and returns the sum without writing to
In
-
This loop should be a function. Also there is no error check
-
The subsequent loop generates a random card that hasn't already been
used up but there should be a
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.
In
In
- 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
voidin 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 arrayand 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) tallyIn
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. Iknow 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
mainmainis too big
- Did you chop out some argument handling in
main? What you have looks odd.
- The reset loop in
mainshould also be extracted into a separate function
- Variable
loopin 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.