Hangman Game (YAHG)

Question

I’m in the process of learning C++ and this was an assignment for a class. It is a simple hangman game. The instructor gives no feedback, only a pass/fail remark. While this works, I would like to know where I can improve. Thank you very much in advance for your help.

/*************************************************************************
* Title: Hangman Game
*
* Write a program that plays the game of Hangman. The program should pick a
* word (which is either coded directly into the program or read from a text
* file) and display the following:
*   Guess the word: XXXXXX
* Each X represents a letter. The user tries to guess the letters in the
* word. The appropriate response yes or no should be displayed after each
* guess. After each incorrect guess, display the diagram with another body
* part filled. After seven incorrect guesses, the user should be hanged. The
* display should look as follows:
*     O
*    /|\
*     |
*    / \
* After each guess, display all user guesses. If the user guesses the word
* correctly, display
*   Congratulations!!! You guessed my word. Play again? yes/no
*************************************************************************/ 
#include <iostream>  // std::cin/std::cout/endl
#include <string>    // std::string
#include <vector>    // vector
#include <ctime>     // time for random seed
#include <random>    // random
#include <cctype>    // isalpha/tolower
#include <algorithm> // transform/count

// Get single character user input.
char getCharInput(const std::string prompt)
{
    char input;

    std::cout << prompt;
    // Input from user.
    std::cin.get(input);
    if (std::cin.peek() != '\n')
        // More than a single char input.
        input = 0;
    // Eat any remaining chars and LF.
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
    return tolower(input);
}

// Check if character is already stored in list of guesses.
bool checkLetter(std::vector<char>& ltrs, const char c, const std::string response)
{
    if (find(ltrs.begin(), ltrs.end(), c) == ltrs.end())
    {
        std::cout << response;
        ltrs.push_back(c);
        return true;
    }
    else
        std::cout << "You already guessed \"" << c << "\"\n";
    return false;
}

int main()
{
    constexpr unsigned numWords{ 15 }, TOP{ 8 }, BASE{ 9 };
    // Lambda random number generator [0...numWords].
    auto rng = [=]() -> unsigned { return int(numWords * rand() / (RAND_MAX + 1.0)); };
    // Seed the rng.
    srand(static_cast<unsigned>(time(0))); rng();
    do {
        // Sample list of words to guess (five each 5, 6 & 7 character long).
        std::string words[numWords] = {
            "blimp", "inuit", "roach", "ankle", "could", "visual", "quartz", "studio", 
            "jockey", "hijack", "located", "alcohol", "crochet", "polymer", "humidor"
        };
        // Characters making up the gallows.
        std::string gallows[] =
        {
            "  |  O\n  | /|\\\n  |  |\n  | / \\", // right leg (complete)
            "  |  O\n  | /|\\\n  |  |\n  | /",    // left leg
            "  |  O\n  | /|\\\n  |  |\n  |",      // torso
            "  |  O\n  | /|\\\n  |\n  |",         // right arm
            "  |  O\n  | /|\n  |\n  |",           // body
            "  |  O\n  | /\n  |\n  |",            // left arm
            "  |  O\n  |\n  |\n  |",              // head
            "  |\n  |\n  |\n  |",                 // empty gallows
            "   ____\n  |  |\n",                  // gallows top
            "\n _|_____\n"                        // gallows base
        };

        std::vector<char> guessedLetters; // List of guessed letters.
        guessedLetters.clear();           // Empty the list.
        unsigned remainingGuesses{ 7 };   // Number of guesses remianing.
        unsigned rn{ rng() };             // Get a random number.
        size_t remainingLetters{ words[rn].size() }; // Number un-guessed letters remaining in word.
        std::string clue = { std::string(words[rn].size(), 'X') }; // Display word clue.

        // Display program purpose.
        std::cout << "Let's play a game of Hangman!\n\n";

        // Loop until word is guessed or user is hung.
        while (remainingLetters && remainingGuesses) {
            // Display clue and get a user guess.
            char g = getCharInput("Guess the word: " + clue + "\n");

            if (isalpha(g)) {
                // Replace 'X' with matching character.
                transform(words[rn].cbegin(), words[rn].cend(), clue.begin(), clue.begin(),
                    [=](char s, char d) { if (s == g) { return g; } else return d; });
            }

            // Check if guessed character had any matches in word.
            if (int matchedLetters = count(clue.cbegin(), clue.cend(), g))
                // Correct guess, decrement remaining letters by proper amount.
                remainingLetters -= (checkLetter(guessedLetters, g, "Good guess!\n") ? matchedLetters : 0);
            else
                // Wrong guess, decrement number of remaining guesses.
                remainingGuesses -= (checkLetter(guessedLetters, g, "Wrong guess!\n") ? 1 : 0);

            // Display results of guess.
            std::cout << gallows[TOP] << gallows[remainingGuesses] << gallows[BASE];

            std::cout << "Letters guessed: ";
            for (char c : guessedLetters)
                std::cout << c << " ";
            std::cout << "\n\n";
        }

        // Display result of game.
        if (!remainingLetters)
        {
            std::cout << "Congratulations!!! You guessed my word \"" << clue;
            std::cout << "\" with " << guessedLetters.size() << " guesses!\n";
        }
        else
            std::cout << "You've been hung!!!\n";

    } while (tolower(getCharInput("Play again? yes/no ")) == 'y');

    return 0;
}

Answer 1

1. Use classes to separate concerns

One improvement could be to introduce your own classes to separate the hangman drawing, from the main game loop.

Something like

class Gallow {
private:
    static std::string gallows[] =
    {
        "  |  O\n  | /|\\\n  |  |\n  | / \\", // right leg (complete)
        "  |  O\n  | /|\\\n  |  |\n  | /",    // left leg
        "  |  O\n  | /|\\\n  |  |\n  |",      // torso
        "  |  O\n  | /|\\\n  |\n  |",         // right arm
        "  |  O\n  | /|\n  |\n  |",           // body
        "  |  O\n  | /\n  |\n  |",            // left arm
        "  |  O\n  |\n  |\n  |",              // head
        "  |\n  |\n  |\n  |",                 // empty gallows
        "   ____\n  |  |\n",                  // gallows top
        "\n _|_____\n"                        // gallows base
    };
    int stage;
public:
    Gallow() : stage(sizeof(gallows)) {}
    void printNextFailedStage() {
        if(stage >= 0) {
            std::cout gallows[stage];
            stage--;
        }
    }
    bool isCompleted() {
        return stage <= 0;
    }
};

Also separating the given words to guess into a class would make the overall designe more flexible.

2. Don't use c-style srand() and rand() functions with c++

Using srand() and rand() functions should be replaced with the Pseudo-random number generation facilities of the current standard.

3. Use specific using statements to reduce namespace references

You can use

using std::cout;

for example to avoid the need to write out std::cout everytime.

Answer 2

Welcome to Code Review! On my first scan through the code I was impressed. I learned a new C++ STL library function, std::transform(). I was really glad to see that there was no using namespace std; statement in the code. Keep up the good work!

Upon executing the program I found one problem:

It might be better to change the prompt at the end of the game to

    } while (tolower(getCharInput("Play again? (y)es/no ")) == 'y');

or accept yes as well as y. I entered yes and the game quit.

Complexity
While he function main() already calls 2 functions, it is still too complex (does too much). As programs grow in size the use of main() should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program.

There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

One of the ideas behind this principle is that the smaller a function is, the easier it is to write, debug, read and maintain.

The outer do while loop could be a function. The inner while loop should be a function. The inner while loop could also add a couple of functions where the comments Replace 'X' with matching character. and // Check if guessed character had any matches in word. are.

In the outer loop I would have created a variable word to simplify the code in the inner loop.

        std::string word = words[rn];

Use of C Style Arrays
There are 2 C style arrays in use, words and gallows. It might be better if these were implemented using a C++ container class such as std::vector or std::array. My choice would be std::vector, at least for words because that would allow the replacement of the symbolic constant numWords with words.size(). Implementing words as a vector also allows additional words to be added to the game easily. If you stick with the current implementation numWords should be declared as std::size_t rather than unsigned, std::size_t is the preferred variable type for array indexes.

The declaration of these 2 arrays should be outside the do while loop because these arrays never change within the scope of the loop. A second reason to move them out of the loop, at least for words is that at some point in the future the contents of words might come from a text file which you would only want to read once.

Declaration of Symbolic Constants
This is a personal style comment. I would define all the symbolic constants outside of any function so that they could be used by all functions, to limit the scope they might be defined as static.

static constexpr std::size_t numWords{ 15 };
static constexpr std::size_t TOP{ 8 };
static constexpr std::size_t BASE{ 9 };

It is considered a good programming practice to define each constant or variable on it's own line to make maintenance and readability easier. It becomes difficult to find declarations defined on one line in large programs.

Variable Names
In the game while loop: - guess might be a better choice than g, single character variable names are not very descriptive.
- rn can only be determined by reading all the code, I'm guessing it means random number.

In the function bool checkLetter(std::vector<char>& ltrs, const char c, const std::string response) it might be easier to understand guessedLetters than ltrs.

Answer 3

You use transform instead of fully qualifying you mean the one from std std::transform leading me to believe there's a user-defined transform function. Try to be specific with the functions you call.