This is a really good effort for a first major project! It's very straightforward and easy to understand. Here are a few ways you could take it to the next level.
Avoid Global Variables
#Avoid Global Variables RightRight now you have 4 global variables:
int balance;
int pot;
int deck[52];
int sdeck[52];
These should be local variables inside the main()
function and you should pass them to the other functions that need to access them. There are 2 reasons:
- As it is now, it's difficult to find who changed them when they change
- If you ever want to expand this (perhaps to be a server that serves games to multiple groups of people at the same time), having globals won't work.
Avoid Magic Numbers
#Avoid Magic Numbers
YouYou created a constant CARDS
but then you only use it once. You should use it to declare your decks, too (and I'd rename it to NUM_CARDS
):
int deck[NUM_CARDS];
int sdeck[NUM_CARDS];
While you're unlikely to ever need to change the number of cards in a deck, using a constant can clarify the code and save you from typos. (And actually, there are a few games that use fewer than 52 cards).
Use Arrays for Looking Things Up
#Use Arrays for Looking Things Up
InIn your csuit()
and cface()
functions, you have a bunch of case
statements to convert between an int
and a string. You could do this more easily with an array, like this:
const char* csuit(const int v)
{
const char* kSuits[] = {
"Hearts",
"Clubs",
"Diamonds",
"Spades"
};
return kSuits [ v % 4 ];
}
You can do a similar thing with cface()
.
Simplify
#Simplify
SeveralSeveral of your functions have complicated nested while
loops. I'd try to simplify them. I'd make the one in main()
it more like this:
while (strcmp(input, "quit") != 0)
{
if (strcmp(input, "play") == 0)
{
play();
}
else if (strcmp(input, "help") == 0)
{
printf("Type 'play' to begin the game.\n");
}
scanf("%s", input);
}
You can eliminate the call to quit()
as your main()
will just exit when the user enters "quit".
On the subject of simplifying, I'd probably also break turn()
into smaller functions, and try to make the logic simpler. Perhaps use a state machine.
This is a really good effort for a first major project! It's very straightforward and easy to understand. Here are a few ways you could take it to the next level.
#Avoid Global Variables Right now you have 4 global variables:
int balance;
int pot;
int deck[52];
int sdeck[52];
These should be local variables inside the main()
function and you should pass them to the other functions that need to access them. There are 2 reasons:
- As it is now, it's difficult to find who changed them when they change
- If you ever want to expand this (perhaps to be a server that serves games to multiple groups of people at the same time), having globals won't work.
#Avoid Magic Numbers
You created a constant CARDS
but then you only use it once. You should use it to declare your decks, too (and I'd rename it to NUM_CARDS
):
int deck[NUM_CARDS];
int sdeck[NUM_CARDS];
While you're unlikely to ever need to change the number of cards in a deck, using a constant can clarify the code and save you from typos. (And actually, there are a few games that use fewer than 52 cards).
#Use Arrays for Looking Things Up
In your csuit()
and cface()
functions, you have a bunch of case
statements to convert between an int
and a string. You could do this more easily with an array, like this:
const char* csuit(const int v)
{
const char* kSuits[] = {
"Hearts",
"Clubs",
"Diamonds",
"Spades"
};
return kSuits [ v % 4 ];
}
You can do a similar thing with cface()
.
#Simplify
Several of your functions have complicated nested while
loops. I'd try to simplify them. I'd make the one in main()
it more like this:
while (strcmp(input, "quit") != 0)
{
if (strcmp(input, "play") == 0)
{
play();
}
else if (strcmp(input, "help") == 0)
{
printf("Type 'play' to begin the game.\n");
}
scanf("%s", input);
}
You can eliminate the call to quit()
as your main()
will just exit when the user enters "quit".
On the subject of simplifying, I'd probably also break turn()
into smaller functions, and try to make the logic simpler. Perhaps use a state machine.
This is a really good effort for a first major project! It's very straightforward and easy to understand. Here are a few ways you could take it to the next level.
Avoid Global Variables
Right now you have 4 global variables:
int balance;
int pot;
int deck[52];
int sdeck[52];
These should be local variables inside the main()
function and you should pass them to the other functions that need to access them. There are 2 reasons:
- As it is now, it's difficult to find who changed them when they change
- If you ever want to expand this (perhaps to be a server that serves games to multiple groups of people at the same time), having globals won't work.
Avoid Magic Numbers
You created a constant CARDS
but then you only use it once. You should use it to declare your decks, too (and I'd rename it to NUM_CARDS
):
int deck[NUM_CARDS];
int sdeck[NUM_CARDS];
While you're unlikely to ever need to change the number of cards in a deck, using a constant can clarify the code and save you from typos. (And actually, there are a few games that use fewer than 52 cards).
Use Arrays for Looking Things Up
In your csuit()
and cface()
functions, you have a bunch of case
statements to convert between an int
and a string. You could do this more easily with an array, like this:
const char* csuit(const int v)
{
const char* kSuits[] = {
"Hearts",
"Clubs",
"Diamonds",
"Spades"
};
return kSuits [ v % 4 ];
}
You can do a similar thing with cface()
.
Simplify
Several of your functions have complicated nested while
loops. I'd try to simplify them. I'd make the one in main()
it more like this:
while (strcmp(input, "quit") != 0)
{
if (strcmp(input, "play") == 0)
{
play();
}
else if (strcmp(input, "help") == 0)
{
printf("Type 'play' to begin the game.\n");
}
scanf("%s", input);
}
You can eliminate the call to quit()
as your main()
will just exit when the user enters "quit".
On the subject of simplifying, I'd probably also break turn()
into smaller functions, and try to make the logic simpler. Perhaps use a state machine.
This is a really good effort for a first major project! It's very straightforward and easy to understand. Here are a few ways you could take it to the next level.
#Avoid Global Variables Right now you have 4 global variables:
int balance;
int pot;
int deck[52];
int sdeck[52];
These should be local variables inside the main()
function and you should pass them to the other functions that need to access them. There are 2 reasons:
- As it is now, it's difficult to find who changed them when they change
- If you ever want to expand this (perhaps to be a server that serves games to multiple groups of people at the same time), having globals won't work.
#Avoid Magic Numbers
You created a constant CARDS
but then you only use it once. You should use it to declare your decks, too (and I'd rename it to NUM_CARDS
):
int deck[NUM_CARDS];
int sdeck[NUM_CARDS];
While you're unlikely to ever need to change the number of cards in a deck, using a constant can clarify the code and save you from typos. (And actually, there are a few games that use fewer than 52 cards).
#Use Arrays for Looking Things Up
In your csuit()
and cface()
functions, you have a bunch of case
statements to convert between an int
and a string. You could do this more easily with an array, like this:
const char* csuit(const int v)
{
const char* kSuits[] = {
"Hearts",
"Clubs",
"Diamonds",
"Spades"
};
return kSuits [ v % 4 ];
}
You can do a similar thing with cface()
.
#Simplify
Several of your functions have complicated nested while
loops. I'd try to simplify them. I'd make the one in main()
it more like this:
while (strcmp(input, "quit") != 0)
{
if (strcmp(input, "play") == 0)
{
play();
}
else if (strcmp(input, "help") == 0)
{
printf("Type 'play' to begin the game.\n");
}
scanf("%s", input);
}
You can eliminate the call to quit()
as your main()
will just exit when the user enters "quit".
On the subject of simplifying, I'd probably also break turn()
into smaller functions, and try to make the logic simpler. Perhaps use a state machine.