7
\$\begingroup\$

I just finished coding a game of 21, similar to Blackjack, but with a simplified dealing mechanism.

#include <iostream>
using namespace std;
#include <string>
#include <ctime>
#include <cstdlib>

int main()
{
    bool loop = true;
    int loopchoice;
    srand(time(0));
    int player1 = 1+(rand()%10);
    int player2 = 1+(rand()%10);
    int banker1 = 1+(rand()%10);
    int banker2 = 1+(rand()%10);
    int playertotal = player1 + player2;
    int bankertotal = banker1 + banker2;
    int choice;
    int playerOVERWRITE;
    int bankerOVERWRITE;
    bool loop1 = true;
    bool loop2 = true;



    cout << "You drew a " << player1 << " and a " << player2 << endl;
    cout << "Your total is " << playertotal << endl;
    cout << "---" << endl;
    cout << "Would you like to 1: Twist or 2: Stick " << endl;
    cin >> choice;
    while( loop1 == true ){
        if( choice == 1 ){
            playerOVERWRITE = 1+(rand()%10);
            playertotal = playertotal + playerOVERWRITE;
            cout << "You drew " << playerOVERWRITE << endl;
            cout << "Your new total is " << playertotal << endl;
            cout << "---" << endl;


        if( playertotal > 21 ){
            loop1 = false;
            cout << "You're bust! Your total is " << playertotal << endl;
            cout << "Bankers total is " << bankertotal << endl;
            cout << "Banker started with a " << banker1 << " and a " << banker2 << endl;
            return 0;
        }
        cout << "Would you like to 1: Twist or 2: Stick " << endl;
        cin >> choice;
        if( choice == 2 ){
            loop1 = false;
            cout << "---" << endl;
        }
        }
        else if( choice == 2 ){
            loop1 = false;
            cout << "---" << endl;
        }

    }
    while( loop2 == true ){
        bankerOVERWRITE = 1+(rand()%10);
        bankertotal = bankertotal + bankerOVERWRITE;
        cout << "Banker drew a " << bankerOVERWRITE << endl;
        cout << "Bankers new total is " << bankertotal << endl;
        cout << "---" << endl;
        if( bankertotal > 21 ){
            cout << "Banker is bust! You win!" << endl;
            cout << "Your total was " << playertotal << endl;
            cout << "Bankers total was " << bankertotal << endl;
            cout << "Banker started with a " << banker1 << " and a " << banker2 << endl;
            cout << " " << endl;
            loop2 = false;
            return 0;
        }
        else if( bankertotal > 18 && bankertotal < 21 || bankertotal == 21 || bankertotal == 18 ){
            cout << "Banker is sticking" << endl;
            cout << "---" << endl;
            loop2 = false;
        }


    }
    if( playertotal > bankertotal ){
        cout << "You win!" << endl;
        cout << "Your total was " << playertotal << endl;
        cout << "Bankers total was " << bankertotal << endl;
        cout << "Banker started with a " << banker1 << " and a " << banker2 << endl;
        cout << " " << endl;
        return 0;
    }
    else if( playertotal == bankertotal ) {
        cout << "It's a draw! Banker wins!" << endl;
        cout << "Your total was " << playertotal << endl;
        cout << "Bankers total was " << bankertotal << endl;
        cout << "Banker started with a " << banker1 << " and a " << banker2 << endl;
        cout << " " << endl;
        return 0;
    }
    else {
        cout << "Banker wins!" << endl;
        cout << "Your total was " << playertotal << endl;
        cout << "Bankers total was " << bankertotal << endl;
        cout << "Banker started with a " << banker1 << " and a " << banker2 << endl;
        cout << " " << endl;
        return 0;
    }

}
\$\endgroup\$
  • 3
    \$\begingroup\$ The Nevada Gaming Commission would like to have a word with you about using 1+(rand()%10) as the dealing mechanism. Rather than closing this question as broken code, I'm editing the question to make it clear that this is not Blackjack. \$\endgroup\$ – 200_success Dec 13 '15 at 15:21
6
\$\begingroup\$

The Player Loop

What we have here is an operation that we want to run once, and then repeat as necessary. That calls for a do...while loop rather than a normal while loop. This will avoid you having to retype your prompt:

do {
    int choice;
    cout << "Would you like to 1: Twist or 2: Stick " << endl;
    cin >> choice;

    if (choice == 1) {
       ...
    }
    else if (choice == 2) {
       break;  // <== this is how we terminate the loop in the non-bust case
    }
    else {
       // maybe print some sort of error here?
    }
} while (true);

The Banker Loop

While for the player, we loop based on the player choices, for the dealerbanker we loop based on a concrete condition: while he's not in range [18, 21]. We don't need an additional variable for this:

while (banker_total < 18) {
    int next = draw_card();
    banker_total += next;
    // ...
}

At the end of this loop, the banker either busted or stuck.

Naming

There are lots of different naming conventions in C++, the most common of which are camelCase and snake_case. I don't have a strong preference between the two, but both are strongly preferred to noseparationwhatsoever. Something like bankertotal is much harder to read than either bankerTotal or banker_total, so prefer to use one or the other.

Next, when you find yourself naming things player1 and player2 - consider that there are two cases: 1 thing and N things. You either need to hold onto one card, or lots of cards. Since you need more than one, it's better to simply have:

std::vector<int> player_cards;
std::vector<int> banker_cards;

This way you can display all the cards that each player had.

\$\endgroup\$
5
\$\begingroup\$

This is very readable code! The indentation seems to be a off, though, inside the first loop. But the naming of the variables is very clear, and that makes the logic easy to follow. My remarks:

Move declaration of variables closer to where they're used. When you do this, you don't have to remember what the type of a variable is. You'll also notice that some variables cannot be moved closer: loop and loopchoice are probably leftovers from rewritten code. When variables are declared at the top, it's easy to miss them when rewriting.

You don't need to print a space. I see you're couting a single space if you want to print an empty line. That's not necessary, you could write cout << endl; as well.

Extract some functions to remove duplicate code. Whenever someone draws a card, the code is always /* var */ = 1+(rand()%10);. You can easily extract this by defining a function above the int main() like so:

int draw_card()
{
    return 1+(rand()%10);
}

Then call it whenever you need a card, for instance:

int player1 = draw_card();

or

playerOVERWRITE = draw_card();

You could also do something like that with printing the choice menu, for instance.

Simplify "== true" tests. if( loop1 == true ){ is the same as if( loop1 ){.

Simplify the first loop. Instead of entering the while( loop1 ){ loop, then testing if( choice == 1){ and then exiting again, you can do something like:

loop1 = ( choice == 1 );
while( loop1 ){
    playerOVERWRITE = draw_card();
    // ...
    cout << "Would you like to 1: Twist or 2: Stick " << endl;
    cin >> choice;
    loop1 = ( choice == 1 );
}
cout << "---" << endl; // This moved outside the loop now

That way, you don't need to have both the if( choice == 2 ){ and the else if( choice == 2 ){ blocks, that do exactly the same.

Simplify the banker's decision.

    else if( bankertotal > 18 && bankertotal < 21 || bankertotal == 21 || bankertotal == 18 ){

means exactly the same as

    else if( bankertotal >= 18 && bankertotal <= 21 ){

Extract the printing of end scores. Even if you do not move the repeated code to a function (which is a good practice), you can move the repeated code at the end out of the if-statements. The end score is printed in each case, anyway.

if( playertotal > bankertotal ){
    cout << "You win!" << endl;
}
else if( playertotal == bankertotal ) {
    cout << "It's a draw! Banker wins!" << endl;
}
else {
    cout << "Banker wins!" << endl;
}
cout << "Your total was " << playertotal << endl;
cout << "Bankers total was " << bankertotal << endl;
cout << "Banker started with a " << banker1 << " and a " << banker2 << endl;
cout << endl;
return 0;

In closing, I really liked reading this code. The spacing is consistent, which makes it pleasant on the eyes, and the intentions are clear, which makes it pleasant on the mind. Keep doing that!

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

I have a few suggestions:

  1. using namespace std; is usually bad practice. Even if you choose to keep it, consider putting it after the include statements.
  2. You sometimes have 3-4 blank lines in your code. This is not necessary, and usually 1 or 2 lines is enough.
  3. Don't put everything in the main method. Consider extracting code into functions.
\$\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.