Code Review for Hangman in C++

I have the following C++ program:

#include 
#include 
#include 
#include 

void printVector (std::vector& vec)
{
    for (int a = 0; a  guessed;
    char userGuess;
    bool letterCorrect;
    while (hangmanPos != 9)
    {
        std::cout > userGuess;
        guessed.push_back(userGuess);
        letterCorrect = false;
        for (int a = 0; a < compWord.size(); a++)
            if (compWord[a] == userGuess)
            {
                if (letterCorrect == false)
                {
                    std::cout << userGuess << " is in my word!\n\n\n";
                    letterCorrect = true;
                }
                compHiddenWord[a] = userGuess;
            }
        if (letterCorrect == false)
        {
            std::cout << "Oops! " << userGuess << " is not in my word.";
            hangmanPos++;
        }
        else
            for (int a = 0; a < compWord.size(); a++)
                if (compWord == compHiddenWord)
                {
                    std::cout << "Well done, " << compWord << " was my word!";
                    return 0;
                }
    }
    std::cout << "\nOh Dear! Looks like you've been hanged. My word was actually " << compWord << ".";
}

Which is supposed to replicate the classic game of hangman visually. Is the code fully optimized? Is there an other way I could improve it?

I'm by far no expert, and I might be wrong on some of my statements, so take them as a basis for discussion. Anyway, I am trying to learn about code quality in C++ myself, so I'm giving it a shot.

• printVector should take the argument as const-ref, because the vector is not supposed to be changed in the function call.

• Consider using an iterator to loop through the vector in printVector. However, in a small-scale program like this I don't see this as a big issue. In larger programs, it might help you to make a function more general. And thanks to C++11's auto, it will be very readable -- even more so if you use a range-based for loop.

• You're using the vector guessed to store the chars from userGuess. Why not a vector to store char? Would also make the type cast to (char) in printVector unnecessary.

• Also you're mixing C-arrays and STL's std::vector, C-strings (char *) and std::string. Try to stick to the STL versions throughout. If you are using a C++11 capable compiler, you can even initialize the std::vector the same way as you're doing with the C-array now.

• You are using curly braces {} in for loops and if/else statements only when absolutely required. Most style guides encourage using braces for every for/if/else block, or at most allow omitting them for one-line "blocks". The reason is readability (you can always expect to find a } where a longer block ends) and editability (It's easy to forget adding the braces when adding a line to the supposed for/if/else block).

• It doesn't look like idiomatic C++ to compare a bool variable with == as in if(letterCorrect == false). You should use something like if (! letterCorrect) instead.

• You should add a return 0; statement when the user is hanged. As far as I know it isn't required for the main function, but as you're already returning with return 0; when the user guesses the word, you should also do so at the end of the function.

• The condition if (compWord == compHiddenWord) is wrong. You're looping over the array elements, but then you're comparing the pointer to the 0th element of the array to the std::string, instead of comparing the array elements. If you follow my suggestion above and change compHiddenWord to std::string, then you can omit the loop and simply write if(compWord == compHiddenWord) (Edit: As I learned, it also works correctly for comparisons of char* and std::string, so I deleted my example for by-character comparison)

• You're using a as a loop index. While this isn't wrong, C++ programmers would typically use i or j for this purpose.

• Edit: Your hangman array goes from 0 to 9 -- but in the while loop you never reach the 9 thanks to the hangManPos != 9 condition. Therefore, you will never print the completely hanged hangman.

Let me know what you think!

StackExchange Code Review Q#25562, answer score: 6