3
\$\begingroup\$

I made a simple BlackJack game and I want to make it as good as possible before I move on to creating a few other games in my "casino" - let me know how I can improve it I added a few things here and there if you are interested my GitHub for this project is linked:

https://github.com/sharoika/CasinoGames

main.cpp :

//
//  main.cpp
//  CasinoGames
//
//  Created by Maksim Sharoika on 2021-07-10.
//

#include <stdlib.h>
#include <iostream>
#include "blackjack.hpp"

// contant for spacer
const int spacer_length = 80;

// function pre-call
int get_input(int, int);
void spacer();

using namespace std;

int main() {
    // variable declarations
    bool game_on = true;
    int money = 100;
    // random seed generation
    srand(unsigned((time(NULL))));
    // introduction message
    cout << "--------------------------------------------------------------------------------" << endl;
    cout << "--------------------------- Welcome to Maximo-Casino ---------------------------" << endl;
    cout << "------------------------ You Have $"<< money << " to Gamble With --------------------------" << endl;
    cout << "--------------------------------------------------------------------------------" << endl;
    
    // game logic
    while (game_on == true && money > 0)
    {
        int game_choice = 0;
        // choice menu of games
        cout << "Please enter the # associated with the game you would like to play, enter 1 to leave." << endl;
        cout << "1. BlackJack" << endl;
        cout << "0. Leave" << endl;
        cout << "Your choice... ";
        
        game_choice = get_input(0, 1);
        
        // leave the casino option
        if (game_choice == 0)
        {
            game_on = false;
        }
        
        // leave blackjack option
        if (game_choice == 1)
        {
            money = blackjack(money);
            cout << "You have returned from backjack with $" << money << "." << endl;
            spacer();
        }
    }
    
    // game end
    if (money <= 0)
    {
        cout << "You lost all your money - come back when you have more." << endl;
    }
    spacer();
    cout << "You are leaving with $" << money << "." << endl;
    cout << "Thank you for playing! - Maxino-Casino." << endl;
    spacer();
    return 0;
}

// space function
void spacer()
{
    for(int i = 0; i < spacer_length; i++)
    {
        cout << "-";
    }
    cout << endl;
}

// input function
int get_input(int low_range, int high_range)
{
    int input;
    while(true)
    {
        cin >> input;
        if (!cin)
        {
            cout << "Invalid response - please try again... ";
            cin.clear();
            cin.ignore(numeric_limits<streamsize>::max(), '\n');
            continue;
        }
        else if(input < low_range || input > high_range)
        {
            cout << "Invalid response - please try again... ";
            cin.clear();
            cin.ignore(numeric_limits<streamsize>::max(), '\n');
            continue;
        }
        break;
      }
    cin.ignore(numeric_limits<streamsize>::max(), '\n');
    cin.clear();
    return input;
}

blackjack.hpp :

//
//  blackjack.hpp
//  CasinoGames
//
//  Created by Maksim Sharoika on 2021-07-11.
//

#ifndef blackjack_hpp
#define blackjack_hpp

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

using namespace std;

// casino functions
int get_input(int, int);
void spacer();

// black-jack function
int blackjack(int money);

// blackjack sub-functions
void restart_cards(int[], int[]);
int get_bet(int);
void generate_cards(int[], int[]);

int ask_player_move();

int calculate_dealer_hand(int, int[]);
int calculate_player_hand(int, int[]);

void inform_player_cards(int, int[]);
void inform_dealer_cards(int, int[]);

void inform_player_total(int);
void inform_dealer_total(int);

int standoff(int, int, int, int);
int bust(int, int, int, int);
int end_game(int, int, int, int);

bool play_again(bool);

#endif /* blackjack_hpp */

blackjack.cpp :

//
//  blackjack.cpp
//  CasinoGames
//
//  Created by Maksim Sharoika on 2021-07-11.
//

// including the header file
#include "blackjack.hpp"

// creating constant varibles
const int max_card_number = 21;

// main BlackJack function
int blackjack(int money)
{
    bool blackjack_game = true;
    int player_card[max_card_number];
    int dealer_card[max_card_number];
    
    spacer();
    cout << "----------------------------- Welcome to BlackJack -----------------------------" << endl;
    cout << "------------------------ You Have $"<< money << " to Gamble With --------------------------" << endl;
    spacer();
    
    // BlackJack game logic
    while(blackjack_game == true)
    {
        // setting up BlackJack round
        restart_cards(player_card, dealer_card);
        int bet = 0;
        int dealer_hand = 0;
        int player_hand = 0;
        int player_move = 1;
        
        // Request Bet from Player
        bet = get_bet(money);
        money = money - bet;
        generate_cards(player_card, dealer_card);
        
        // Dealer's Section
        spacer();
        cout << "Dealer's Hand: "<< endl;
        cout << "Card #1: " << "###" << endl;
        cout << "Card #2: " << dealer_card[1] << endl;
        spacer();
        
        // player move logic
        for(int i = 2; player_hand < 22 && player_move == 1; i++)
        {
            player_hand = calculate_player_hand(i, player_card);
            inform_player_cards(i, player_card);
            inform_player_total(player_hand);
            money = bust(player_hand, dealer_hand, money, bet);
            if(player_hand < 22)
            {
            player_move = ask_player_move();
            }
        }
        
        // dealer move logic
        if(player_hand < 22)
        {
            int dealer_card_number = 2;
            dealer_hand = calculate_dealer_hand(dealer_card_number, dealer_card);
            for(int i = 2; dealer_hand < 17; i++)
            {
                dealer_hand = calculate_dealer_hand(i+1, dealer_card);
                dealer_card_number++;
            }
            dealer_hand = calculate_dealer_hand(dealer_card_number, dealer_card);
            inform_dealer_cards(dealer_card_number, dealer_card);
            inform_dealer_total(dealer_hand);
            money = bust(player_hand, dealer_hand, money, bet);
        }
        
        // end game logic
        if(dealer_hand < 22 && player_hand < 22)
        {
            money = end_game(money, bet, player_hand, dealer_hand);
        }
        
        // play again logic
        if(money > 0)
        {
            blackjack_game = play_again(blackjack_game);
        }
        else
        {
            blackjack_game = false;
        }
        
    }
    return money;
}

void restart_cards(int player_card[], int dealer_card[])
{
    for(int i = 0; i < max_card_number; i++)
    {
        dealer_card[i] = 0;
        player_card[i] = 0;
    }
}

int get_bet(int money)
{
    int local_bet;
    cout << "You currently have: $" << money << endl;
    cout << "How much would you like to bet... $";
    local_bet = get_input(0, money);
    return local_bet;
}

void generate_cards(int player_card[], int dealer_card[])
{
    for(int i = 0; i < max_card_number; i++)
    {
        player_card[i] = ((rand() % 13) + 1);
        dealer_card[i] = ((rand() % 13) + 1);
        if(player_card[i] > 10)
        {
            player_card[i] = 10;
        }
        if(dealer_card[i] > 10)
        {
            dealer_card[i] = 10;
        }
    }
}

int ask_player_move()
{
    int input;
    cout << "What would you like your move to be?" << endl;
    cout << "1. Hit " << endl;
    cout << "2. Stand" << endl;
    cout << "Please enter the numerical value associated... ";
    input = get_input(1, 2);
    spacer();
    return input;
}

int calculate_dealer_hand(int number_of_hits, int dealer_card[])
{
    int dealer_hand = 0;
    for(int i = 0; i < number_of_hits; i++)
    {
        dealer_hand = dealer_hand + dealer_card[i];
    }
    return dealer_hand;
}

int calculate_player_hand(int number_of_hits, int player_card[])
{
    int player_hand = 0;
    for(int i = 0; i < number_of_hits; i++)
    {
        player_hand = player_hand + player_card[i];
    }
    return player_hand;
}


void inform_player_cards(int current_cards, int player_card[])
{
    cout << "Your have hit " << current_cards - 2 << " time(s)." << endl;
    cout << "Player's Hand: " << endl;
    for(int i = 0; i < current_cards; i++)
    {
        cout << "Card #" << i+1 << ": " << player_card[i] << endl;
    }
}

void inform_dealer_cards(int current_cards, int dealer_card[])
{
    cout << "Dealer has hit " << current_cards - 2 << " time(s)." << endl;
    cout << "Dealer's Hand: " << endl;
    for(int i = 0; i < current_cards; i++)
    {
        cout << "Card #" << i+1 << ": " << dealer_card[i] << endl;
    }
}

void inform_dealer_total(int dealer_hand)
{
    cout << "The dealer's total card value is: " << dealer_hand << endl;
    spacer();
}

void inform_player_total(int player_hand)
{
    cout << "Your player's card value is: " << player_hand << endl;
    spacer();
}

int standoff(int money, int bet, int player_hand, int dealer_hand)
{
    if (player_hand == 21)
    {
        cout << "Both are BlackJacks money returned.";
        money = money + bet;
    }
    else
    {
        cout << "Dealer wins because no one has blackjacks.";
        money = money;
    }
    return money;
}

int bust(int player_hand, int dealer_hand, int money, int bet)
    {
    if(player_hand > 21)
    {
        cout << "PLAYER BUSTED." << endl;
        money = money;
        cout << "You now have $" << money << "." << endl;
        spacer();
    }
    if(dealer_hand > 21)
    {
        cout << "DEALER BUSTED." << endl;
        money = money + 2*bet;
        cout << "You now have $" << money << "." << endl;
        spacer();
    }
    return money;
}

int end_game(int money, int bet, int player_hand, int dealer_hand)
{
    if(player_hand == dealer_hand)
    {
        cout << "Your and the dealer have a 'Stand Off'" << endl;
        money = standoff(money, bet, player_hand, dealer_hand);
    }
    else if(player_hand < dealer_hand)
    {
        cout << "DEALER WINS." << endl;
    }
    else if(player_hand > dealer_hand)
    {
        cout << "PLAYER WINS." << endl;
        money = money + 2*bet;
    }
    cout << "You now have $" << money << "." << endl;
    spacer();
    return money;
}

bool play_again(bool blackjack_game)
{
    cout << "Would you like to play again?" << endl;
    cout << "1. Yes" << endl;
    cout << "0. No" << endl;
    cout << "Please enter numerical value...";
    blackjack_game = get_input(0, 1);
    spacer();
    return blackjack_game;
}

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

You already have a good code review from JDługosz; I am going to give you a higher level design review instead.

When I look at your code, the first thing that strikes me is that I see no classes anywhere. That’s a code smell; it tells me that what I’m looking at is not actually C++ code, but rather C code. (Or, at best, “C++--”). The reason the lack of classes is such a problem is that the whole point of writing code in a high (or mid) level language like C++ is to model your problem. That’s literally the reason Stroustrup invented the language: he wanted to model his problem and C just couldn’t do it (and the languages that could were too slow).

So let’s look at how you’ve implemented Blackjack:

int blackjack(int money)
{
    // ... [snip] ...
    int player_card[max_card_number];
    int dealer_card[max_card_number];

    // ... [snip] ...

    // ... actual game happens in this loop block ...
    {
        // ... clear the hands (because you're reusing them each game (not good!)) ...

        // ... [snip] ...

        // ... get the player's bet ...

        // ... generate the two hands ...

        // ... report the dealer's first card ...

        // ... get the player move (hit or stand) ...

        // ... calcuate the dealer move ...

        // ... figure out who won ...

        // ... ask whether to play again (and restart the loop) ...
    }

    return money;
}

Now the fishy thing about this structure is the ... generate the two hands ... part. The way you’ve implemented the game is that you’ve given both players hands with 21 cards, and then you randomly generate the scores for those cards all in one shot at the start of the game, and slowly reveal them to the player as the game goes on.

That’s not how Blackjack works.

Blackjack is not played by giving both the player and the dealer 21 cards face down, and then turning up one card at a time. Because you’ve coded it that way, you’ve introduced all kinds of problems and absurdities. For example, it is possible—unlikely, but possible—that generate_cards() gives both the player and the dealer 21 “2” cards. Can you imagine how that game would play out?

Dealer: 2 ?
Player: (total = 4) 2 2

Player hits

================================

Dealer: 2 ?
Player: (total = 6) 2 2 2

Player hits

================================

Dealer: 2 ?
Player: (total = 8) 2 2 2 2

Player thinks: WTF? *Five* twos?
Player hits

================================

Dealer: 2 ?
Player: (total = 10) 2 2 2 2 2

Player thinks: Are you freaking kidding me? *Six* twos?
Player hits

================================

Dealer: 2 ?
Player: (total = 12) 2 2 2 2 2 2

Player thinks: Okay, this is ridiculous.
Player hits

================================

Dealer: 2 ?
Player: (total = 14) 2 2 2 2 2 2 2

Player thinks: ...
Player hits several more times

================================

Dealer: 2 ?
Player: (total = 20) 2 2 2 2 2 2 2 2 2 2

Player thinks: ... wow ...
Player stands
Dealer now hits a bunch of times

================================

Dealer: (total = 18) 2 2 2 2 2 2 2 2 2
Player: (total = 20) 2 2 2 2 2 2 2 2 2 2

Player wins.
Yay?

Basically the key point of Blackjack—the whole point of the strategy of the game—is that the player is able to look at the cards that have been dealt and calculate odds on what might come up next. But that won’t work here, because you’re just generating random numbers with no respect for what’s left in the deck.

In other words, this is not really Blackjack. It’s a random number guessing game.

Let’s take a step back.

What is Blackjack? It’s a card game; it’s a game played with cards. So you probably need a card type:

struct card_t {};

What is a card? Well, a card has a suit and a rank:

struct card_t
{
    suit_t suit;
    rank_t rank;
};

I won’t go further into the design of cards, suits, ranks, decks and hands, because I’ve already written about that elsewhere. But once you have types for all these things, you just need to add BlackJack-specific logic. In BlackJack, scoring a hand isn’t as simple as just adding up the card values, because aces have two scores: 1 or 11. So when scoring a hand, you need to return multiple possible scores.

For example, if your hand is 🃑🂴, the score is either 5 or 15. This is where a large part of the excitement and strategy comes in. Do I stand with 15? Do I try to hit and hope I get a 6 or less? Do I want to risk getting a 7–9 and my score going down? I can look at the cards that are visible and calculate percentages. THAT is what the game is all about.

So when you’re scoring a hand, you need to calculate potential multiple scores, and return the list of scores. If all the possible scores of a hand are greater than 21, that’s a bust. If any possible score is 21, you should stop there. If all possible scores are less than 21, then you could either hit or stand depending on a number of factors that the player and your game AI could take into account. For that, you need a scoring function that returns a list of scores. Obviously the minimum number of scores a hand may have is 1… but what is the maximum? Well, if your hand has all 4 aces in the deck, then there are 5 possible scores:

Score Ace 1 Ace 2 Ace 3 Ace 4
4 1 1 1 1
14 1 1 1 11
24 1 1 11 11
34 1 11 11 11
44 11 11 11 11

More generally, the number of possible scores for a hand is the number of aces in the hand plus one. And each possible score is just plus ten.

So you could calculate scores like this:

constexpr auto number_of_aces_in_deck = std::size_t{4};

// rough function, just slapped together, no error checking
auto score_blackjack_hand(hand_t const& hand)
{
    auto score_without_aces = 0;
    auto number_of_aces_in_hand = 0;
    for (auto&& card : hand)
    {
        switch (card.rank)
        {
        case rank_t::ace:
            ++number_of_aces;
            break
        case rank_t::jack:
            [[fallthrough]];
        case rank_t::queen:
            [[fallthrough]];
        case rank_t::king:
            score += 10;
            break;
        default;
            score += static_cast<int>(card.rank);
            break;
        }
    }

    auto scores = std::array<int, number_of_aces_in_deck + 1>{};
    scores.fill(score_without_aces + number_of_aces_in_hand);

    auto p = scores.begin();
    for (auto i = 0; i < number_of_aces_in_hand; ++i)
        std::for_each(++p, scores.end(), [](auto&& score) { score += 10; });

    return scores;
}

This returns an array of possible scores, from lowest to highest (the highest score is repeated because this function just returns an array… you should really use a proper type for this).

Now the game is basically this:

auto deck = deck_t::full_deck(); // sets up your full deck of 52 cards
deck.shuffle();

auto player_hand = deck.deal(2); // takes 2 cards from the deck to create a new deck (used as the player hand)
auto dealer_hand = deck.deal(2);


while (player_is_not_done)
{
    // ... do the betting ...

    std::cout << "you hand is: " << player_hand << '\n';
    std::cout << "the dealer is showing: " << dealer_hand[0] << '\n';

    std::cout << "hit or stand?\n";

    if (player_wants_hit)
    {
        player_hand.draw_from(deck, 1);

        // check minimum score of player's hand
        if (score_blackjack_hand(player_hand)[0] > 21)
        {
            // player busts
            player_is_not_done = false;
        }
    }
    else
    {
        player_is_not_done = false;
    }
}

// did the player bust?
if (score_blackjack_hand(player_hand)[0] > 21)
{
    std::cout << "you're busted, dealer wins\n";
}
else
{
    std::cout << "dealer's hand: " << dealer_hand << '\n';

    // get_highest_score() gets the highest score in a set of possible scores
    // that is less than or equal to 21, or if all possible scores are greater
    // than 21, just returns the lowest score

    while (get_highest_score(dealer_hand) < get_highest_score(player_hand) and dealer_hand[0] <= 17)
    {
        std::cout << "dealer draws\n";
        dealer_hand.draw_from(deck, 1);
        std::cout << "dealer's hand: " << dealer_hand << '\n';
    }

    if (dealer_hand[0] > 21)
    {
        std::cout << "dealer busts; you win\n";
    }
    else if (get_highest_score(dealer_hand) < get_highest_score(player_hand))
    {
        std::cout << "you win\n";
    }
    else
    {
        std::cout << "dealer wins\n";
    }
}

The really powerful thing about doing things this way is that:

  1. It’s automatically correct (assuming you got the model right). There will be no absurdities like being able to draw 19 twos.
  2. If you want to modify/extend the game, you can do so naturally. For example, if you want to add another player to the game above, it’s pretty trivial (the only “complication” is that you now need to check multiple scores, but the code above is just slapped together; if I had properly designed it, I would have considered the possibility of multiple players already… I might have created an abstract player class that would allow human players locally and remotely, and multiple types of AI players so people could play against some computer players that are risk-takers, others that are conservative, etc.).

That’s why we design by modelling the system we’re simulating, rather than just hacking together something that sorta-kinda “looks” right.

So I would suggest you start by looking at the system—the game of Blackjack­. What are the elements of the game? (Well, there are cards, hands, the deck, players, scores, and, if you want betting, money.) How do those elements interact? (Well, players have hands of cards, they draw cards from the deck, and so on.) What is the logic of the system? (Well, scores are calculated by the card’s face value, except face cards are scored at 10, and aces can be 1 or 11. If a hand’s minimum score is 21, the hand busts, and so on.)

Once you’ve done all that, model the system in C++. Create types for cards, decks, hands, the players, and so on. Make those types work and interact just like the actual things they are modelling.

If you do all that, and if you get it right, the actual game code will flow very naturally, it will be very hard to introduce bugs or logic errors, and it will be easy to extend or modify the game.

(You could even take all the above to a higher level, and instead of just thinking about Blackjack, instead think about the whole casino, where Blackjack is only one game of many. At that level, you would be modelling entire games as entities. For example, you might have an abstract game base class, and concrete games like Blackjack derive from that. Then you might be able to do things like keep a history of the games the player has played, so they can see where they’re losing the most money.)

That is what programming in C++ is really like. It’s not just writing C, except using std::cout instead of printf(). It’s about thinking in terms of modelling real-world systems and their interactions in code.

I am not going to do a code review, because JDługosz already did that. But I will say that your code is archaic; you are not writing C++, you are writing C (with maybe one or two C++ bells and whistles, like std::cout, but for just that there’s really no point). It’s not just that there’s not a single type in your code (which is really bad, because C++ is not just a strongly-typed language, it is the strongly-typed language… C++ is all about the types). It’s a bunch of things, like:

  • Using C arrays instead of C++ arrays.
  • Using bare ints for everything instead of dedicated types for scores, money, etc.. (This is why you have function signatures like this: int end_game(int, int, int, int). That’s just horrifying. What do all those ints mean? It’s way too easy to forget the order and mix things up.)
  • Using naked loops instead of algorithms. Algorithms are already tested, optimized, and safe, and since they’re named, it’s a lot clearer what they’re doing.

Also, there’s no testing at all. That’s very bad. Untested code is garbage code; you may object by saying “but it works!” to which I’d say… “does it? prove it”. I wouldn’t trust untested in any project I’m working on. You need to get in the habit of properly testing your code; a good practice is to write the tests first, and then write your code… that way you won’t be tempted to skip the tests, and writing the tests first can really help you design your code better because they tests will show you what you actually need.

So test your code!

\$\endgroup\$
1
  • \$\begingroup\$ Wonderful - Thank you. \$\endgroup\$
    – Max
    Aug 7 at 21:31
1
\$\begingroup\$
#include <stdlib.h>

Why are you including the deprecated C header? Why do you need the C header <stdio.h> as well as <iostream>? You would use <cstdlib> if you really needed those C library features, but I think this is just for the old rand stuff, and you should look into <random> header instead.

// contant for spacer
const int spacer_length = 80;

Spelling? Did you mean "constant"?
That would be a singularly useless comment.

For the constant itself, prefer constexpr


// function pre-call
int get_input(int, int);
void spacer();

What is a "pre-call"? I think you mean you are declaring the functions that will be defined later in the file? That is inferior to just defining them in the proper order. Due to overloading, having this declaration not match the subsequent definition will not be noticed as an error.

In any case, you also declare them in the HPP file, which is where they belong. Declaring them again here, when you already included the header file, doesn't add anything.


Don’t write using namespace std;.

You can, however, in a CPP file (not H file) or inside a function put individual using std::string; etc. (See SF.7.)

Especially, never do that in a header file! You should not pollute the scope of whatever file includes this header, and this is an especially egregious example of polluting the scope. A header file should not cause any modifications to the includer beyond the API it is defining.


    // variable declarations
    bool game_on = true;
    int money = 100;

Your comment makes it seem like you're clustering all the local variables in a specific section. You don't do that — declare variables where needed. You don't need a comment to say you're declaring a variable, as the statement is itself a variable declaration. These are mixed in with other statements so it would be silly to use such a comment each time!


void restart_cards(int[], int[]);
int get_bet(int);
void generate_cards(int[], int[]);

Just looking at the functions in the header, I wonder what are those parameters? Note that this syntax is not actually passing an array, but is automatically converted to a pointer. There's no lengths given, nor any names. Many of the functions have the same or similar parameters, so I wonder if that really ought to be a class with member data?


while(blackjack_game == true)
Don't write explicit comparisons against true or false. You already have a bool value. The operator== between bools produces another bool, so would you have to test that too? I mean, you don't write (x==true)==true right? It's the same thing: you already have a bool that represents what you want to do.


        int bet = 0;
            ⋮
            ⋮
        // Request Bet from Player
        bet = get_bet(money);

This is an example of what I mentioned earlier: declare variables when you are ready to use them. This should be:

const auto bet = get_bet(money);

That is, defined where it is ready to get its proper value, and hopefully made const as well.


money = money - bet;

Note that you can write: money -= bet;


Don't use endl. There are numerous long explanation posts on this board already. Just use the \n character where you want a line break.


void restart_cards(int player_card[], int dealer_card[])
{
    for(int i = 0; i < max_card_number; i++)
    {
        dealer_card[i] = 0;
        player_card[i] = 0;
    }
}

Learn what's already there in the standard library! This is just fill_n.

This is an example too of there being no proper encapsulation: you're passing pointers to the start of arrays to these functions, but those arrays are fixed variables and have specific purposes, and you are not associating the length with the pointer. They should be member data and the various functions that use them can refer to the names directly and not have to be passed.

Why do you need restart_cards anyway? That zeros everything, but you never leave it in such a state, but call generate_cards to populate with the real values.


        player_card[i] = ((rand() % 13) + 1);
        dealer_card[i] = ((rand() % 13) + 1);
        if(player_card[i] > 10)
        {
            player_card[i] = 10;
        }
        if(dealer_card[i] > 10)
        {
            dealer_card[i] = 10;
        }
    }

Don't repeat the same code with only the variable changed. Abstract it out into a function. You would have something like: player_card[i] = deal_card();.

This is more like dice, not cards: rather than a shuffled deck, it picks a random value every time.

This function, generate_cards, is doing the same thing for two different arguments. Why not make the function have one copy of the code, and then call it twice with two different arguments? That is:

generate_cards (dealer_card);
generate_cards (player_card);

int calculate_dealer_hand(int number_of_hits, int dealer_card[])
{
    int dealer_hand = 0;
    for(int i = 0; i < number_of_hits; i++)
    {
        dealer_hand = dealer_hand + dealer_card[i];
    }
    return dealer_hand;
}

int calculate_player_hand(int number_of_hits, int player_card[])
{
    int player_hand = 0;
    for(int i = 0; i < number_of_hits; i++)
    {
        player_hand = player_hand + player_card[i];
    }
    return player_hand;
}

I think you are not understanding what parameters are for!

You have two identical functions, that are each called with a different (fixed) variable when you use it. Each call should be passing the data it wants to operate on to the same piece of code.

There are other examples of this throughout.

As for this function in particular, it took me a while to figure out how it's supposed to work. You're adding up the entire hand each time you add a card, rather than keeping the total and just dealing another card to it. The "number of hits" is the total number of cards dealt, which you have to keep track of specifically, and separate from the cards array.


Good things to continue

I see you separated the input into its own functions, which is a good thing.

You have made functions for many different steps, which looks like you did some kind of "top-down design". That's good. However, your main function is not decomposed enough; it calls a lot of small functions with meaningful names, but each "clump" is a cohesive set of logic that should be its own function. Basically, each header-like comment introduces a section that should be a function, and that comment would be the function name.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Great points - I will be back with all the suggestions sometimes... :) Thanks so much! \$\endgroup\$
    – Max
    Aug 7 at 21:31

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.