4
\$\begingroup\$

Normally I code in Java but have recently been trying to teach myself C for a university course I will be taking next semester. (I'm about to enter my third year). I wouldn't consider myself the best programmer, or even a good one.

Now, I was just wondering if anyone would be able to take a quick look through my attempt at making a terminal based Blackjack game in C and tell me if there are any glaring holes in how I've made it. I'm not used to writing C or larger procedural code projects for that matter, so I'm sure I've done a lot of silly things. I've included the main.c file below, but here is the GitHub link for the rest of it.

#include <stdio.h>
#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include "data.h"
#include "deck.h"

#define MAX_CARDS 25

Hand deal_player_hand(Deck deck);
Card* get_random_card(Deck deck);
Hand deal_dealer_hand(Deck deck);
void print_card(Card* card);
void print_hand(Hand player_hand);
char* suite_to_string(int suite);
void hit(Hand player_hand, Deck deck);
char get_user_response();
bool check_valid(char input);
Card* deal_card(Hand player_hand, Deck deck);
bool check_bust(Hand hand);
void dealer_hit(Hand dealer_hand, Deck deck);
int sum_hand(Hand hand);
void check_winner(Hand player_hand, Hand dealer_hand);


int main() {

    Deck deck = create_deck();
    Hand player_hand = deal_player_hand(deck);
    Hand dealer_hand = deal_dealer_hand(deck);

    printf("\nPlayer Hand:\n");
    print_hand(player_hand);
    printf("\nDealer Hand:\n");
    print_hand(dealer_hand);

    hit(player_hand, deck);
    dealer_hit(dealer_hand, deck);
    check_winner(player_hand, dealer_hand);

    return 0;
}

void dealer_hit(Hand dealer_hand, Deck deck) {

    int sum;

    do {
        printf("\nThe dealer deals themself a card\n");
        printf("Dealer Hand:\n");
        deal_card(dealer_hand, deck);
        print_hand(dealer_hand);
        sum = sum_hand(dealer_hand);
    } while(sum < 16);

    if (sum <= 21) {
        printf("\nThe dealer's hand is above 16, they must sit.");
    } else if (sum > 21) {
        printf("\nThe dealer has gone bust!\n");
    }
}

Hand deal_player_hand(Deck deck) {

    Hand player_hand = malloc(sizeof(Card*) * MAX_CARDS); // not possible to draw more than 25 cards

    for (int i = 0; i < MAX_CARDS; i++) {
        player_hand[i] = NULL;
    }

    player_hand[0] = get_random_card(deck);
    player_hand[1] = get_random_card(deck);

    return player_hand;
}

Hand deal_dealer_hand(Deck deck) {

    Hand dealer_hand = malloc(sizeof(Card*) * MAX_CARDS); // not possible to draw more than 25 cards

    for (int i = 0; i < MAX_CARDS; i++) {
        dealer_hand[i] = NULL;
    }

    dealer_hand[0] = get_random_card(deck);

    return dealer_hand;
}

Card* get_random_card(Deck deck) {

    srand(time(NULL));

    int random_index;

    do {
        random_index = rand() % DECK_SIZE;
    } while (deck[random_index]->dealt == true);

    Card* random_card = deck[random_index];
    random_card->dealt = true;
    return random_card;
}

// deals a random card to the next available slot in player or dealer's hand
Card* deal_card(Hand hand, Deck deck) {

    Card* card = NULL;

    for (int i = 0; i < MAX_CARDS; i++) {
        if (hand[i] == NULL) {
            card = get_random_card(deck);
            hand[i] = card;
            break;
        }
    }

    return card;
}

void hit(Hand player_hand, Deck deck) {

    bool stop_hitting = false;

    while (!stop_hitting && !check_bust(player_hand)) {
        printf("\nHit? (y/n)\n");
        char input = get_user_response();
        if (input == 'n') {
            stop_hitting = true;
        } else {
            Card* card = deal_card(player_hand, deck);
            print_card(card);
        }
    }

    if (check_bust(player_hand)) {
        printf("You've gone bust\n");
        printf("Game Over\n");
        exit(0);
    }

    printf("Player Hand:\n");
    print_hand(player_hand);
}

void check_winner(Hand player_hand, Hand dealer_hand) {

    int player_sum = sum_hand(player_hand);
    int dealer_sum = sum_hand(dealer_hand);

    printf("\nSum of Player's hand: %d\n", player_sum);
    printf("Sum of Dealer's hand: %d\n", dealer_sum);

    if (player_sum == dealer_sum) {
        printf("It's a draw!\n");
    } else if (player_sum < dealer_sum) {
        printf("The house wins!\n");
    } else if (player_sum > dealer_sum) {
        printf("You win!\n");
    }
}

//###########################
// GRAPHICS FUNCTIONS
//###########################

void print_card(Card* card) {
    char* suite = suite_to_string(card->suite);
    printf("%s of %s\n", card->name, suite);
}

void print_hand(Hand player_hand) {

    int value = 0;

    for (int i = 0; i < MAX_CARDS; i++) {
        Card* card = player_hand[i];

        if (card == NULL) {
            break;
        }

        value += card->value;
        print_card(card);
    }
    printf("Sum: %d\n", value);
}

//###########################
// UTILITY FUNCTIONS
//###########################
char* suite_to_string(int suite) {

    char *suite_name = malloc(sizeof(char) * 10);

    switch (suite) {
        case DIAMONDS:
            strcpy(suite_name, "Diamonds");
            break;
        case CLUBS:
            strcpy(suite_name, "Clubs");
            break;
        case SPADES:
            strcpy(suite_name, "Spades");
            break;
        case HEARTS:
            strcpy(suite_name, "Hearts");
            break;
        default:
            strcpy(suite_name, "ERROR");
    }

    return suite_name;
}

bool check_valid(char input) {

    bool valid = false;

    if (input == 'y' || input == 'n') {
        valid = true;
    }

    return valid;
}

// determines if this hand has gone over 21.
bool check_bust(Hand hand) {

    bool bust = false;
    int sum = sum_hand(hand);

    if (sum > 21) {
        bust = true;
    }

    return bust;
}

int sum_hand(Hand hand) {

    int sum = 0;

    for (int i = 0; i < MAX_CARDS; i++) {
        if (hand[i] != NULL) {
            sum += hand[i]->value;
        }
    }

    return sum;
}

//###########################
// File IO Functions
//###########################

// gets first char entered by user on command line
char get_user_response() {

    char input;
    scanf(" %c", &input);
    while(!check_valid(input)) {
        printf("Invalid input. Please enter y/n\n");
        scanf(" %c", &input);
    }

    return input;
}

Thank you in advance for any suggestions you may have!

\$\endgroup\$
  • 1
    \$\begingroup\$ For a beginner that looks pretty good actually. There's some parts that look very rough, but that's to be expected if this is your first real C project. \$\endgroup\$ – Mast Jan 17 at 6:39
  • 1
    \$\begingroup\$ Could you include the definitions of Card, Deck and Hand, please? The code won't compile without them. \$\endgroup\$ – Toby Speight Jan 17 at 8:11
6
\$\begingroup\$

Good first effort. Coming from Java, you'll find memory management in C new and frustrating:

Memory management

You can't just call malloc and directly use the result like this:

Hand player_hand = malloc(sizeof(Card*) * MAX_CARDS);

for (int i = 0; i < MAX_CARDS; i++) {
    player_hand[i] = NULL;
}

If malloc() fails, it will return a null pointer, and that will cause Undefined Behaviour when we reach player_hand[i] = NULL;. It's vital that we check the result before we use it, perhaps as simple as:

Hand player_hand = malloc(sizeof(Card*) * MAX_CARDS);
if (!player_hand)
    fputs("Memory allocation error!\n", stderr);
    exit(EXIT_FAILURE);
}

Another thing that's new to Java programmers is the need to free() the memory we allocate. For a small program like this, we get away with not releasing our memory, as it will all be reclaimed by our OS at program exit, but we'll want to develop good practice for longer-running programs (perhaps we'll want to develop a game server built around this code, for example).

There's a simple rule: every allocation must be paired with a deallocation somewhere in the program. A large part of C programming is concerned with managing allocations and ensuring there's clear ownership of each one until it is released.

Error checking

As well as malloc(), there are other functions whose return value indicates errors and must therefore be checked. One such example is scanf(), such as here:

char input;
scanf(" %c", &input);
while(!check_valid(input)) {
    printf("Invalid input. Please enter y/n\n");
    scanf(" %c", &input);
}

scanf() returns the number of conversions performed, or EOF if there was an I/O failure. What happens if there is a failure? (We can force failure, just by closing the input stream.) input doesn't get assigned, so its value is uninitialised. Unless it happens to contain a valid response, then we'll loop indefinitely, failing to read input each time. To fix this, we need to inspect the return value (I'll reorder the loop, so we only need to code this once):

char get_user_response(void)
{
    char input;
    while (scanf(" %c", &input) == 1) {
        if (check_valid(input)) {
            return input;
        }
        printf("Invalid input. Please enter y or n\n");
    }
    fputs("Input error!\n", stderr);
    exit(EXIT_FAILURE);
}

Choose which strings are writable

We have

char* suite_to_string(int suite);

If we look where we use this, we never need to write to the returned memory - we just use it for printing. This means we can return a const char* instead, which in turn means that we don't need to allocate memory: we can just return a pointer to a string literal:

#include <assert.h>
#define NOTREACHED(message) 0

const char *suite_to_string(int suite)
{
    switch (suite) {
    case DIAMONDS: return "Diamonds";
    case CLUBS: return "Clubs";
    case SPADES: return "Spades";
    case HEARTS: return "Hearts";
    }
    assert(NOTREACHED("Invalid suit name"));
    return "ERROR";
}

If we were to declare an enum for the suit, then a good compiler would check that we'd included all the values in the switch (using default would prevent that, which is why I've put the error-handling outside the block).

\$\endgroup\$
  • \$\begingroup\$ I'd also advice to use calloc() instead of malloc() in this case. \$\endgroup\$ – G. Sliepen Jan 17 at 9:31
  • \$\begingroup\$ I wouldn't - we immediately write null pointers to all the storage allocated, so zeroing the memory is just busy-work. (In fact, I was glad not to see an assumption that all-zero would necessarily be a null pointer) \$\endgroup\$ – Toby Speight Jan 17 at 9:42
  • 1
    \$\begingroup\$ I was thinking that it would avoid having to have that loop to NULL all members of the array. But you are right, the NULL pointer might not be all zeroes (though I have yet to encounter this mythical machine where this is the case). \$\endgroup\$ – G. Sliepen Jan 17 at 10:17
  • \$\begingroup\$ Thank you so much! I had no idea you could return a const char* straight from a function without having to malloc anything, very useful. I have one question though, you say malloc can sometimes give NULL if there is an error, how come you check this with if (!player_hand) instead of if (player_hand == NULL)? Do they both work? \$\endgroup\$ – connor-myers Jan 17 at 13:22
  • 1
    \$\begingroup\$ Yes, the ! operator on a pointer is exactly equivalent to comparing with literal 0 (which converts to a null pointer). It's a common C idiom to test pointers for validity like that. \$\endgroup\$ – Toby Speight Jan 17 at 13:38
2
\$\begingroup\$

There's a subtle issue with this line in the get_random_card function:

srand(time(NULL));

You're seeding the random number generator with the current time, each time get_random_card is called, which means multiple calls within the same second will have the same sequence of random numbers. In this specific case, it doesn't affect anything (other than a small performance penalty) because of the do ... while (deck[random_index]->dealt == true);, which will loop until it gets to the next number in the sequence anyway. If you change the implementation though, or use random numbers somewhere else in the program, you could run into subtle bugs where the numbers aren't as random as you think they are.

You should be calling srand(time(NULL)) once at the beginning of the program, then leave the seed alone in get_random_card.

\$\endgroup\$
1
\$\begingroup\$

Spelling

"Suite" is not the same as "Suit". A suit would be Hearts, Clubs, Spades and Diamonds.

Logic Checking

  • Why is MAXCARDS 25? In Blackjack, the most cards a player is allowed is 5. Even then, the target is 21; with a standard deck of cards, that would be 4 Twos (8), 3 Threes (9) and 4 Aces (4). If you are planning on playing with more than 1 deck, then you can adjust the numbers as necessary, but the game is supposed to stop dealing cards to a player once 5 cards are there.
  • I don't see the logic for handling cards and their values. I assume that's in your Github link (Sorry, I didn't follow it). Ensure that Aces are treated as both 11 and 1 - when I do blackjack, I treat them as 11 until the player busts, then convert to a value of 1, and re-run the check.

Code Style

  • void print_hand(Hand player_hand); would indicate to me that ONLY the player_hand would be passed through here, but you also pass the dealer's hand through this function. Good, you aren't repeating yourself - but I suggest changing it to Hand hand to indicate that it will run the check on any valid Hand that gets passed to it
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.