5
\$\begingroup\$

For my display functions in my Blackjack game, I want to control each player's stats output based on whether a turn is in progress or has ended. Normally, you could only see one card that your opponent(s) hold, while the rest are only revealed at the end of each turn. My code does that well, but I'm having trouble making it more concise.

The display function in Game is called to display each player's stats (name, chips, and hand). It takes a Boolean argument to determine if it should display each CPU's partial stats (during each turn) or full stats (after each turn). At the function call in Game, the bool argument is passed to the respective display function in Player, where the human player and CPU player have individual display procedures. The display functions in Player primarily display the chip amounts and card rank totals. They also call respective Hand display functions which, again, solely depend on the Boolean argument for displaying the CPUs' stats.

How could I simplify this code to achieve the same output (shown below the code)?

Game.cpp

displayStats(false); // function call elsewhere in Game

void Game::displayStats(bool showCPUResults) const
{
    players[0].displayPlayerStats(); // displays player stats

    for (int i = 1; i < numPlrs; i++)
        players[i].displayCPUStats(showCPUResults); // displays CPU stats
}

Player.cpp

void Player::displayPlayerStats() const
{
    // always shown
    std::stringstream ss;
    std::cout << "\n* " << name << ": ";
    std::cout << "($" << chips << ") ";

    // displays two-digit card rank total (always shown)
    if (totalCardsValue <= 9)
        ss << "(0" << totalCardsValue << ")  ";
    else
        ss << "(" << totalCardsValue << ")  ";

    std::cout << ss.str();

    // displays human player's hand
    playerHand[0].displayPlayerHand();
}

void Player::displayCPUStats(bool showResults) const
{
    // always shown
    std::stringstream ss;
    std::cout << "\n* " << name << ": ";
    std::cout << "($" << chips << ") ";

    if (!showResults)
    {
        std::cout << "      ";
        playerHand[0].displayCPUHand(showResults);
    }
    else
    {
            // only shown after each turn
        if (totalCardsValue <= 9)
            ss << "(0" << totalCardsValue << ")  ";
        else
            ss << "(" << totalCardsValue << ")  ";

        std::cout << ss.str();
            
            // displays CPU's hand
        playerHand[0].displayCPUHand(showResults);
    }
}

Hand.cpp

void Hand::displayPlayerHand() const
{
    // displays player's full hand (always)
    for (unsigned i = 0; i < cards.size(); i++)
        std::cout << cards[i] << " ";
}

void Hand::displayCPUHand(bool showCPUHand) const
{
    // displays CPU's first card (always)
    std::cout << cards[0] << " ";

    // displays the rest of the CPU's cards (only after each turn)
    if (showCPUHand)
    {
        for (unsigned i = 1; i < cards.size(); i++)
            std::cout << cards[i] << " ";
    }
}

Output during a turn...

Jamal: ($1000) (23) [8C] [6H] [9C]

CPU1: ($1000) [AD]

...and after a turn...

Jamal: ($1000) (23) [8C] [6H] [9C]

CPU1: ($1000) (21) [AD] [KH]

\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

If you want to make things more consice, you could change a few things:

  • Number of card printed on 2 digits:

    if (totalCardsValue <= 9)
        ss << "(0" << totalCardsValue << ")  ";
    else
        ss << "(" << totalCardsValue << ")  ";
    

    could be written

    ss << ((totalCardsValue <= 9) ? "(0" : "(") << totalCardsValue << ")  ";
    

    or

    ss << "(" << std::setw(2) << std::setfill('0') << totalCardsValue << ")  ";
    
  • No need for temporary std::stringstream. I think you could just std::cout directly without using std::stringstream ss.

  • Removing duplicated code: void Hand::displayPlayerHand() does the same as void Hand::displayCPUHand(true). This would probably better if you were naming this method display and calling the parameter displayAllCards or displayOnlyFirst. Also, you might want to give it a default value.

    Corresponding code for Hand.cpp is:

    void Hand::display(bool displayAllCards) const 
    {
        // displays the first card
        std::cout << cards[0] << " ";
    
        // displays the rest of the cards if required
        if (displayAllCards)
        {
            for (unsigned i = 1; i < cards.size(); i++)
                std::cout << cards[i] << " ";
        }
    }
    
  • Removing duplicated code: things can be changed in Player.cpp too as Player::displayCPUStats and Player::displayPlayerStats really look alike.

    Edited to finish my answer: Indeed, displayPlayerStats() is nothing but displayCPUStats(true). Here again, it might be worth changing the names of methods and variables.

    Corresponding code for Player.cpp is:

    void Player::displayStats(bool displayAllCards) const
    {
        std::cout << "\n* " << name << ": ";
        std::cout << "($" << chips << ") ";
    
        if (displayAllCards)
        {
            std::cout << ((totalCardsValue <= 9) ? "(0" : "(") << totalCardsValue << ")  ";
        }
        playerHand[0].displayHand(displayAllCards);
    }
    
\$\endgroup\$
4
  • \$\begingroup\$ Yes, I too am looking at Player::displayPlayerStats and Player::displayCPUStats and trying to "merge" them. As for Hand::displayCompHand, it's actually Hand::displayCPUHand. I changed that part of the name but must've missed that instance. Sorry about that! Knowing that, you may continue what you were about to do (and I'll change that bit in the code now). \$\endgroup\$
    – Jamal
    Apr 8, 2013 at 21:22
  • \$\begingroup\$ I've just finished. \$\endgroup\$
    – SylvainD
    Apr 8, 2013 at 22:32
  • \$\begingroup\$ And I was just about to answer my question (alongside yours) since I've already found an answer. :-) Who should post it, then? \$\endgroup\$
    – Jamal
    Apr 8, 2013 at 22:34
  • \$\begingroup\$ Okay, I take back my answer; yours is more concise than mine. I really need to start practicing this. Thanks again! \$\endgroup\$
    – Jamal
    Apr 8, 2013 at 22:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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