patterncppModerate
Testing new C++11 features with Hangman
Viewed 0 times
newwithhangmantestingfeatures
Problem
I wrote a Hangman game to try out some of the new features in C++11. I'm pretty new to C++, and I would like some good advice on how I can improve this code (in terms of conventions, bad/good practices, ...):
```
#include
#include
#include
#include
#include
#include
#include
const unsigned initial_number_of_lives = 5;
// Returns a random word from the given words file.
std::string pick_word(const std::string& words_filename) {
std::vector words;
std::ifstream words_file(words_filename.c_str());
if (!words_file) {
throw std::runtime_error("Couldn't open file.");
}
std::string word;
do {
std::getline(words_file, word);
words.push_back(word);
} while (!words_file.eof());
std::uniform_int_distribution random_distribution(0, words.size() - 1);
std::mt19937 random_engine(static_cast(std::time(nullptr)));
uintmax_t word_index = random_distribution(random_engine);
return words[word_index];
}
int main(int argc, const char *argv[]) {
if (argc != 2) {
std::cerr \n";
return 1;
}
std::string word;
try {
word = pick_word(argv[1]);
} catch(...) {
std::cerr guessed_letters;
for (;;) {
bool won = true;
for (char &letter : word) {
if (std::find(guessed_letters.begin(),
guessed_letters.end(),
letter) == guessed_letters.end()) {
won = false;
}
}
if (won) {
std::cout > guess;
guess = std::tolower(guess);
// Don't allow player to enter the same letter twice.
if (std::find(guessed_letters.begin(),
guessed_letters.end(),
guess) != guessed_letters.end()) {
std::cout << "You have already guessed that letter!\n";
continue;
}
// Don't allow player to enter anything but letters.
if (!std::isalpha(guess)) {
std::cout << "That is not a letter!\n";
continue;
}
guessed_letters.push_back(guess);
bool word_contains_letter = false;
for (char &le
```
#include
#include
#include
#include
#include
#include
#include
const unsigned initial_number_of_lives = 5;
// Returns a random word from the given words file.
std::string pick_word(const std::string& words_filename) {
std::vector words;
std::ifstream words_file(words_filename.c_str());
if (!words_file) {
throw std::runtime_error("Couldn't open file.");
}
std::string word;
do {
std::getline(words_file, word);
words.push_back(word);
} while (!words_file.eof());
std::uniform_int_distribution random_distribution(0, words.size() - 1);
std::mt19937 random_engine(static_cast(std::time(nullptr)));
uintmax_t word_index = random_distribution(random_engine);
return words[word_index];
}
int main(int argc, const char *argv[]) {
if (argc != 2) {
std::cerr \n";
return 1;
}
std::string word;
try {
word = pick_word(argv[1]);
} catch(...) {
std::cerr guessed_letters;
for (;;) {
bool won = true;
for (char &letter : word) {
if (std::find(guessed_letters.begin(),
guessed_letters.end(),
letter) == guessed_letters.end()) {
won = false;
}
}
if (won) {
std::cout > guess;
guess = std::tolower(guess);
// Don't allow player to enter the same letter twice.
if (std::find(guessed_letters.begin(),
guessed_letters.end(),
guess) != guessed_letters.end()) {
std::cout << "You have already guessed that letter!\n";
continue;
}
// Don't allow player to enter anything but letters.
if (!std::isalpha(guess)) {
std::cout << "That is not a letter!\n";
continue;
}
guessed_letters.push_back(guess);
bool word_contains_letter = false;
for (char &le
Solution
A few thoughts:
Instead of
You should do
Otherwise, the read which causes the stream to go into an invalid state will still be pushed.
Also, why not include the filename that cannot be opened in the exception text and then output the exception text instead of the hard-coded text you have now. As things are, you may be ignoring other exceptions (although I don't see when one would happen). Perhaps output the exception.what() in addition to what you have now.
I would also advise splitting some of the game logic into functions -- you don't need them in multiple places as things are, but it could be easier to follow. Things like the piece setting
Oh, another thing: you may want to use
Instead of
do {
std::getline(words_file, word);
words.push_back(word);
} while (!words_file.eof());You should do
while(std::getline(words_file, word))
words.push_back(word);
}Otherwise, the read which causes the stream to go into an invalid state will still be pushed.
Also, why not include the filename that cannot be opened in the exception text and then output the exception text instead of the hard-coded text you have now. As things are, you may be ignoring other exceptions (although I don't see when one would happen). Perhaps output the exception.what() in addition to what you have now.
I would also advise splitting some of the game logic into functions -- you don't need them in multiple places as things are, but it could be easier to follow. Things like the piece setting
word_contains_letter would look better as function calls, in my opinion.Oh, another thing: you may want to use
#ifndef NDEBUG rather than #ifdef DEBUG, as that is the default way to check whether debugging is enabled or not.Code Snippets
do {
std::getline(words_file, word);
words.push_back(word);
} while (!words_file.eof());while(std::getline(words_file, word))
words.push_back(word);
}Context
StackExchange Code Review Q#7362, answer score: 10
Revisions (0)
No revisions yet.