Code Review of Hangman Game

Hi all. I'm new to the forums, and have been working on this game for awhile. It works fine, but I feel like I could improve on it even more. For now, however, I would like to ask for you all to look over this and make sure I haven't over complicated anything or broken any general conventions in my code. Anyways, thank you for your time and review.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
#include <iostream>
#include <string>
#include <vector>
#include <algorithm>
#include <fstream>
#include <ctime>
#include <sstream>
#include <cctype>
#include <iterator>
#include <cstring>

void prompt(const int& rMaxWrongGuesses, int& rWrongGuesses, std::string& rGuessedLetters, std::string& rSoFar, std::string& rHint);
char getGuess(std::string& rGuessedLetters, const std::string& rTheWord, std::string& rSoFar, int& rWrongGuesses);
void checkGuess(char guess, const std::string& rTheWord, std::string& rSoFar, int& rWrongGuesses);
void shutDown(int& rWrongGuesses, const int& rMaxWrongGuesses, const std::string& rTheWord);
void drawGallows(int& rWrongGuesses);

int main()
{
    //setup
    const int kMaxWrongGuesses = 8; //maximum number of incorrect guesses allowed


    // Gets list of words from file and assigns them to vector by line
    std::ifstream myfile("words.txt"); // Formatted as such: WORD,_THIS_IS_A_DEFINITION_
    std::vector<std::string> words((std::istream_iterator<std::string>(myfile)),
                                   std::istream_iterator<std::string>());

    // Shuffles  Words
    std::mt19937 g(std::time(0));
    std::shuffle(words.begin(), words.end(), g);

    // Split Words and Definitions
    std::stringstream ss(words[0]);
    std::string item;
    char delim = ',';
    while (std::getline(ss, item, delim)) {
        words.push_back(item);
    }

    // Find all characters after a comma in kTheWord and erase them.
    std::string kTheWord = words[0];
    kTheWord = kTheWord.substr(0, kTheWord.find(",", 0));

    // Initialize Hint and strip underscores used in file
    std::string hint = words.back();
    std::replace(hint.begin(), hint.end(), '_', ' ');

    // Set Variables used in game
    int wrongGuesses = 0;
    std::string soFar(kTheWord.size(), '-');
    std::string GuessedLetters = "";
    char guess;

    // References for performance and ease of access
    const int& rMaxWrongGuesses = kMaxWrongGuesses;
    int& rWrongGuesses = wrongGuesses;
    const std::string& rTheWord = kTheWord;
    std::string& rGuessedLetters = GuessedLetters;
    std::string& rSoFar = soFar;
    std::string& rHint = hint;

  std::cout << "Welcome to Hangman. Good luck!\n";

  while ((wrongGuesses < kMaxWrongGuesses) && (soFar != kTheWord))
  {
    prompt(rMaxWrongGuesses, rWrongGuesses, rGuessedLetters, rSoFar, rHint);
    char guess = getGuess(rGuessedLetters, rTheWord, rSoFar, rWrongGuesses);
    checkGuess(guess, rTheWord, rSoFar, rWrongGuesses);
    drawGallows(rWrongGuesses);
  }
  shutDown(rWrongGuesses, rMaxWrongGuesses, rTheWord);
}

void prompt(const int& rMaxWrongGuesses, int& rWrongGuesses, std::string& rGuessedLetters, std::string& rSoFar, std::string& rHint)
{
  std::cout << "\n\nYou have " << (rMaxWrongGuesses - rWrongGuesses);
  std::cout << " incorrect guesses left.\n";
  std::cout << "\nYou've used the following letters:\n" << rGuessedLetters << std::endl;
  std::cout << "\nSo far, the word is:\n" << rSoFar << std::endl;
  std::cout << "and your hint is: " << rHint << " \n";
}

char getGuess(std::string& rGuessedLetters, const std::string& rTheWord, std::string& rSoFar, int& rWrongGuesses)
{
  char guess;
  std::cout << "\n\nEnter your guess: ";
  std::cin >> guess;
  guess = toupper(guess); // make uppercase since secret word is in uppercase
  while (rGuessedLetters.find(guess) != std::string::npos)
  {
    std::cout << "\nYou've already guessed " << guess << std::endl;
    std::cout << "Enter your guess: ";
    std::cin >> guess;
    guess = toupper(guess);
  }

  rGuessedLetters += guess;
  return guess;
}

void checkGuess(char guess, const std::string& rTheWord, std::string& rSoFar, int& rWrongGuesses)
{
  if (rTheWord.find(guess) != std::string::npos)
  {
    std::cout << "That's right! " << guess << " is in the word.\n";

    //update soFar to include newly guessed letter
    for (int i = 0; i < rTheWord.length(); ++i)
    {
      if (rTheWord[i] == guess)
      {
        rSoFar[i] = guess;
      }
    }
  }
  else
  {
    std::cout << "Sorry, " << guess << " isn't in the word.\n";
    ++rWrongGuesses;
  }
}

void shutDown(int& rWrongGuesses, const int& rMaxWrongGuesses, const std::string& rTheWord)
{
  if (rWrongGuesses == rMaxWrongGuesses)
  {
    std::cout << "\nYou've been hanged!";
  }
  else
  {
    std::cout << "\nYou guessed it!";
  }

  std::cout << "\nThe word was " << rTheWord << std::endl;
  system("pause");
}

void drawGallows(int& rWrongGuesses)
{
  if(rWrongGuesses==8)
  {
    std::cout << "\n\n";
    std::cout << "   +----+     "<<"\n";
    std::cout << "   |    |     "<<"\n";
    std::cout << "   |    O     "<<"\n";
    std::cout << "   |   /|\\   "<<"\n";
    std::cout << "   |   / \\   "<<"\n";
    std::cout << "   |          "<<"\n";
    std::cout << "  ============"<<"\n\n";;
  }
  else if(rWrongGuesses==7)
  {
    std::cout <<"\n\n";
    std::cout <<"   +----+  "<<"\n";
    std::cout <<"   |    |  "<<"\n";
    std::cout <<"   |    O  "<<"\n";
    std::cout <<"   |   /|\\ "<<"\n";
    std::cout <<"   |     \\ "<<"\n";
    std::cout <<"   |       "<<"\n";
    std::cout <<"  ============"<<"\n\n";;
  }
  else if(rWrongGuesses==6)
  {
    std::cout <<"\n\n";
    std::cout <<"   +----+  "<<"\n";
    std::cout <<"   |    |  "<<"\n";
    std::cout <<"   |    O  "<<"\n";
    std::cout <<"   |   /|\\ "<<"\n";
    std::cout <<"   |       "<<"\n";
    std::cout <<"   |       "<<"\n";
    std::cout <<"  ============="<<"\n\n";;
  }
  else if(rWrongGuesses==5)
  {
    std::cout <<"\n\n";
    std::cout <<"   +----+  "<<"\n";
    std::cout <<"   |    |  "<<"\n";
    std::cout <<"   |    O  "<<"\n";
    std::cout <<"   |   /|  "<<"\n";
    std::cout <<"   |       "<<"\n";
    std::cout <<"   |       "<<"\n";
    std::cout <<"  ============="<<"\n\n";;
  }
  else if(rWrongGuesses==4)
  {
    std::cout  <<"\n\n";
    std::cout  <<"   +----+  "<<"\n";
    std::cout  <<"   |    |  "<<"\n";
    std::cout  <<"   |    O  "<<"\n";
    std::cout  <<"   |    |  "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"  ============="<<"\n\n";;
  }
  else if(rWrongGuesses==3)
  {
    std::cout  <<"\n\n";
    std::cout  <<"   +----+  "<<"\n";
    std::cout  <<"   |    |  "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"  ============="<<"\n\n";;
  }
  else if(rWrongGuesses==2)
  {
    std::cout  <<"\n\n";
    std::cout  <<"   +----+  "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"  ============="<<"\n\n";;
  }
  else if(rWrongGuesses==1)
  {
    std::cout<<"\n\n";
    std::cout  <<"   +       "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"   |       "<<"\n";
    std::cout  <<"  ============="<<"\n\n";;
  }
  else {};
}
Your code has several problems that you should fix:
In function 'int main()': 53:10: warning: unused variable 'guess' [-Wunused-variable] At global scope: 84:64: warning: unused parameter 'rTheWord' [-Wunused-parameter] 84:87: warning: unused parameter 'rSoFar' [-Wunused-parameter] 84:100: warning: unused parameter 'rWrongGuesses' [-Wunused-parameter] In function 'void checkGuess(char, const string&, std::string&, int&)': 109:23: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]


Also what is the purpose of the following:
1
2
3
4
5
6
7
    // References for performance and ease of access
    const int& rMaxWrongGuesses = kMaxWrongGuesses;
    int& rWrongGuesses = wrongGuesses;
    const std::string& rTheWord = kTheWord;
    std::string& rGuessedLetters = GuessedLetters;
    std::string& rSoFar = soFar;
    std::string& rHint = hint;

Whoops, I moved the call of checkGuess() outside of getGuess() and forgot to delete the extra parameters. As for that section of code, I passed the variables as references to reduce overhead, or at least that was my intention.
You don't need to create reference variables to pass variables by reference. In your function prototype and implementation you use the ampersand (&) to indicate that the parameter is a reference. In the function call you just use the variable names.
1
2
3
4
5
6
7
8
// Prototype:
void someFunction(int& number, int mult);

...
int value = 0;
int size = 10;
...
someFunction(value, size); // value is passed by reference, size is passed by value. 

Also trying to add some kind of type identifier is considered a bad practice (rWrongGuesses, kMaxWrongGuesses), doing so will probably just confuse yourself later.







Oh, thank you. That cleared up the code a lot. As for the type identifier, Google's C++ Style Guide recommends the "k" in front of constants and I've been recently updating and trying to write my code to follow that in order to make it more consistent. Did you see anything else that needed fixing or could be improved?
As for the type identifier, Google's C++ Style Guide recommends the "k" in front of constants

Okay, but with that style guide you only need the 'k' for constants when using global or static constants. Variables local to your functions don't require this annotation.
All such variables with static storage duration (i.e. statics and globals, see Storage Duration for details) should be named this way. This convention is optional for variables of other storage classes, e.g. automatic variables, otherwise the usual variable naming rules apply.

IMO if you keep your functions small you really don't need these kind of annotations for the local variables.

By the way I hope you realize that using non-const reference parameters is prohibited by this style guide. This is one of the reasons I don't particularly like this style guide, since they disallow many of the more modern C++ features and instead rely on the "tried and true" C style features.

...... and I've been recently updating and trying to write my code to follow that in order to make it more consistent.


Although it's a good thing to have a consistent style, there are many style guides which are much better than the Google style guide. Stroustrup's one can be found here:

http://www.stroustrup.com/JSF-AV-rules.pdf

Note they disallow exceptions, but that is because of the very real need (life critical) for these to happen in a guaranteed amount of time, which they can't do.

There also lots of other really good things to read:

https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines.html#main
https://isocpp.org/faq
https://akrzemi1.wordpress.com/

Topic archived. No new replies allowed.