Sign in to follow this  
NUCLEAR RABBIT

Ending to Early

Recommended Posts

Hello, I finished my hangman game today, but there is one more problem left. When the user plays the game, it works fine, but after the player has either won or lost, if he selects to play another game, it stars, but when a user selects to enter a letter, it ends. Thank you for any Help. Hangman Source:
#include <ctime>
#include <string>
#include <vector>
#include <iostream>
#include <algorithm>
#include <iterator>

using namespace std;

class Player
{
      public:
             Player(const string TheWord);
             ~Player(); 
             string SetWord(); 
             void SetGuess(); 
             void GetUsed(); 
             void GetSoFar(); 
             void GetNumGuesses(const string& TheWord);
             void CheckWin(const string& TheWord, bool& win, bool& done);
             void CheckGuess(const string& TheWord); 
             static int m_NumGuesses; 
      private:
             char m_Guess;
             vector<char> * m_Used;
             string * m_SoFar;
}; 

int Menu();
void Pause();
string SetWord();

int Player::m_NumGuesses = 0;

int main()
{
    string TheWord = SetWord();
    
    bool done = false;
    bool win = false;
    
    while(!done)
    {
        TheWord = SetWord();
        Player * player = new Player(TheWord);
        
        switch(Menu())
        {
            case 0:
                cout << "\n\nBye!\n";
                Pause();
                done = true;
                break;
            case 1:
                do
                {
                    player->GetUsed();
                    player->GetNumGuesses(TheWord);
                    player->GetSoFar();
                    player->SetGuess();
                    player->CheckGuess(TheWord);
                    player->CheckWin(TheWord, win, done);
                }while(!win);
                break;
            default:
                cout << "\n\nInvalid Option.\n\n";
                Pause();
        }
    }
    
    return 0;
}

//------------------------------------------------------------------------------
// Name: GetNumGuesses
// Desc: Prints the amount of guesses left
//------------------------------------------------------------------------------

void Player::GetNumGuesses(const string& TheWord)
{
    cout << "\nAmount of Guesses left: " << (TheWord.size() - m_NumGuesses)
         << endl;
}

//------------------------------------------------------------------------------
// Name: CheckWin()
// Desc: checks to see if user has won
//------------------------------------------------------------------------------

void Player::CheckWin(const string& TheWord, bool& win, bool& done)
{
    if(*m_SoFar == TheWord)
    {
        cout << "\n\nYou Won!! The word was " << TheWord << "\n";
        Pause();
        win = true;
    }
    if((TheWord.size() - m_NumGuesses) <= 0)
    {
        cout << "\n\nYou Lost!! The word was " << TheWord << "\n";
        Pause();
        win = true;
    }
}

//------------------------------------------------------------------------------
// Name: GetSoFar()
// Desc: Prints out the letters guesses correcty so far
//------------------------------------------------------------------------------

void Player::GetSoFar()
{
    cout << "\nThe Word So Far: " << *m_SoFar << endl << endl;
}

//------------------------------------------------------------------------------
// Name: CheckGuess()
// Desc: Checks to see if the users guess is correct
//------------------------------------------------------------------------------

void Player::CheckGuess(const string& TheWord)
{
    bool check = false;
    
    do
    {
         for(int i = 0; i != TheWord.size(); ++i)
         {
                 if(TheWord.find(m_Guess) == string::npos)
                 {
                      m_Used->push_back(m_Guess);
                      check = true;
                      m_NumGuesses++;
                      break;
                 }
                 else if(m_Guess == TheWord[i])
                 {
                      (*m_SoFar)[i] = m_Guess;
                      check = true;
                 }
         }
    }while(!check);
}

//------------------------------------------------------------------------------
// Name: GetUsed()
// Dsc: Prints out the letters that have been guessed
//------------------------------------------------------------------------------

void Player::GetUsed()
{
    system("CLS");
    
    cout << "\nUsed Letters: "; 
    
     for(vector<char>::const_iterator cIter = m_Used->begin(); cIter != m_Used->end(); ++cIter)
     {
          cout << " " << *cIter << " ";
     }
     
     cout << endl;
}

//------------------------------------------------------------------------------
// Name: SetGuess()
// Desc: Gets the users guess
//------------------------------------------------------------------------------

void Player::SetGuess()
{    
     cout << "\nPlease enter a letter guess: ";
     cin >> m_Guess;
}

//------------------------------------------------------------------------------
// Name: Menu()
// Desc: Gets the user's desired action
//------------------------------------------------------------------------------

int Menu()
{
    system("CLS");
    
     int menu= 0;
     
     cout << "Welcome to Hangman\n"
          << "Programed by Brandon Wall\n\n"
          << "Menu Items:\n\n"
          << "0 = Quit\n"
          << "1 = New Game\n\n"
          << "Menu Selection: ";
     cin >> menu;
     
     system("CLS");
     
     return menu;
}

//------------------------------------------------------------------------------
// Name: Pause()
// Desc: Waits for the user to continue
//------------------------------------------------------------------------------

void Pause()
{
     cout << "\nPress [ENTER] to continue.";
     cin.ignore();
     cin.get();
}

//------------------------------------------------------------------------------
// Name: Player()
// Desc: Allocates memry on the heap for the private vectors
//------------------------------------------------------------------------------

Player::Player(string TheWord)
{
    m_Used = new vector<char>;
    m_SoFar = new string(TheWord.size(), '-');
}

//------------------------------------------------------------------------------
// Name: ~Player()
// Desc: Free's all the memory on the heap
//------------------------------------------------------------------------------

Player::~Player()
{
    delete m_Used;
    delete m_SoFar;
}

//------------------------------------------------------------------------------
// Name: SetWord()
// Desc: Sets the word the user to guess
//------------------------------------------------------------------------------

string SetWord()
{
        srand(time(NULL));
    
       vector<string> words;
       words.push_back("buiness");
       words.push_back("computer");
       words.push_back("evergreen");
       words.push_back("designer");
       words.push_back("dynamite");
       
       random_shuffle(words.begin(), words.end());
       
       return words[0];
}

Share this post


Link to post
Share on other sites
... !

I will require several passes in order to illustrate all the problems here.

Pass 1: don't allocate dynamic memory where you don't need it. It's just asking for memory leaks (here, assuming a working program, you would leak the player object, and actually its members as well, because the destructor doesn't get called) and adds complexity that accomplishes exactly nothing. Please keep in mind that when you use a standard library container such as a vector, it is *already* putting things on the heap *for you*, which is a Good Thing. In the rare cases where you do need to allocate memory explicitly, you should follow the standard library's example, by designing your class in such a way that it can be used *as if* it were all on the stack. That's one form of "information hiding", you know.

As well, you can put short functions inline in the class declaration (that removes emphasis from the implementations when they're obvious). Similarly, you should use initializer lists to initialize data members; and do not declare or implement destructors if the default will suffice. I'm only cleaning up the ctor and dtor for now because I'll be completely redoing the class design anyway.

Oh, and while you do effectively have to prototype your class member functions (by writing the class body), I strongly recommend that instead of prototyping free functions at the top and defining them later, instead put them in order such that they only have to be prototyped where necessary (because of circular dependencies). By doing it that way, you expose said circular dependencies (because you will be forced to handle them) and save typing. C++ offers you this horrible one-pass compilation model; instead of fighting it with extra boilerplate, try to reap the few benefits. I also put the Player member function implementations next to the Player class body, so that they're all in one chunk (you should consider moving such things to a separate header/source file pair).

With that, plus my reformatting (you could at least be consistent about indentation amount), we get:


#include <ctime>
#include <string>
#include <vector>
#include <iostream>
#include <algorithm>
#include <iterator>

using namespace std;

class Player {
public:
Player(const string& TheWord) : m_SoFar(TheWord.size(), '-') {}

string SetWord();
void SetGuess();
void GetUsed();
void GetSoFar();
void GetNumGuesses(const string& TheWord);
void CheckWin(const string& TheWord, bool& win, bool& done);
void CheckGuess(const string& TheWord);
static int m_NumGuesses;

private:
char m_Guess;
vector<char> m_Used;
string m_SoFar;
};

int Player::m_NumGuesses = 0;

//------------------------------------------------------------------------------
// Name: GetNumGuesses
// Desc: Prints the amount of guesses left
//------------------------------------------------------------------------------
void Player::GetNumGuesses(const string& TheWord) {
cout << "\nAmount of Guesses left: " << (TheWord.size() - m_NumGuesses) << endl;
}

//------------------------------------------------------------------------------
// Name: CheckWin()
// Desc: checks to see if user has won
//------------------------------------------------------------------------------
void Player::CheckWin(const string& TheWord, bool& win, bool& done) {
if (m_SoFar == TheWord) {
cout << "\n\nYou Won!! The word was " << TheWord << "\n";
Pause();
win = true;
}
if ((TheWord.size() - m_NumGuesses) <= 0) {
cout << "\n\nYou Lost!! The word was " << TheWord << "\n";
Pause();
win = true;
}
}

//------------------------------------------------------------------------------
// Name: GetSoFar()
// Desc: Prints out the letters guesses correcty so far
//------------------------------------------------------------------------------

void Player::GetSoFar() {
cout << "\nThe Word So Far: " << m_SoFar << endl << endl;
}

//------------------------------------------------------------------------------
// Name: CheckGuess()
// Desc: Checks to see if the users guess is correct
//------------------------------------------------------------------------------

void Player::CheckGuess(const string& TheWord) {
bool check = false;
do {
for (int i = 0; i != TheWord.size(); ++i) {
if (TheWord.find(m_Guess) == string::npos) {
m_Used.push_back(m_Guess);
check = true;
m_NumGuesses++;
break;
} else if(m_Guess == TheWord[i]) {
m_SoFar[i] = m_Guess;
check = true;
}
}
} while (!check);
}

//------------------------------------------------------------------------------
// Name: GetUsed()
// Dsc: Prints out the letters that have been guessed
//------------------------------------------------------------------------------

void Player::GetUsed() {
system("CLS");
cout << "\nUsed Letters: ";

for(vector<char>::const_iterator cIter = m_Used.begin(); cIter != m_Used.end(); ++cIter) {
cout << " " << *cIter << " ";
}

cout << endl;
}

//------------------------------------------------------------------------------
// Name: SetGuess()
// Desc: Gets the users guess
//------------------------------------------------------------------------------

void Player::SetGuess() {
cout << "\nPlease enter a letter guess: ";
cin >> m_Guess;
}

// END OF PLAYER CLASS

//------------------------------------------------------------------------------
// Name: Menu()
// Desc: Gets the user's desired action
//------------------------------------------------------------------------------
int Menu() {
system("CLS");

int menu = 0;
cout << "Welcome to Hangman\n"
<< "Programed by Brandon Wall\n\n"
<< "Menu Items:\n\n"
<< "0 = Quit\n"
<< "1 = New Game\n\n"
<< "Menu Selection: ";
cin >> menu;

system("CLS");

return menu;
}

//------------------------------------------------------------------------------
// Name: Pause()
// Desc: Waits for the user to continue
//------------------------------------------------------------------------------
void Pause() {
cout << "\nPress [ENTER] to continue.";
cin.ignore();
cin.get();
}

//------------------------------------------------------------------------------
// Name: SetWord()
// Desc: Sets the word the user to guess
//------------------------------------------------------------------------------
string SetWord() {
srand(time(NULL));

vector<string> words;
words.push_back("business"); // fixed typo here ;)
words.push_back("computer");
words.push_back("evergreen");
words.push_back("designer");
words.push_back("dynamite");

random_shuffle(words.begin(), words.end());

return words[0];
}

int main() {
string TheWord = SetWord();

bool done = false;
bool win = false;

while (!done) {
TheWord = SetWord();
Player player(TheWord);

switch (Menu()) {
case 0:
cout << "\n\nBye!\n";
Pause();
done = true;
break;

case 1:
do {
player.GetUsed();
player.GetNumGuesses(TheWord);
player.GetSoFar();
player.SetGuess();
player.CheckGuess(TheWord);
player.CheckWin(TheWord, win, done);
} while (!win);
break;

default:
cout << "\n\nInvalid Option.\n\n";
Pause();
}
}
return 0;
}



I'm out this evening, so more later; rest assured I will eventually fix the actual bug, but there's too much other stuff I have to comment on first, so that you actually learn something [wink]

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
... !

I will require several passes in order to illustrate all the problems here.

Pass 1: don't allocate dynamic memory where you don't need it. It's just asking for memory leaks (here, assuming a working program, you would leak the player object, and actually its members as well, because the destructor doesn't get called) and adds complexity that accomplishes exactly nothing. Please keep in mind that when you use a standard library container such as a vector, it is *already* putting things on the heap *for you*, which is a Good Thing. In the rare cases where you do need to allocate memory explicitly, you should follow the standard library's example, by designing your class in such a way that it can be used *as if* it were all on the stack. That's one form of "information hiding", you know.

As well, you can put short functions inline in the class declaration (that removes emphasis from the implementations when they're obvious). Similarly, you should use initializer lists to initialize data members; and do not declare or implement destructors if the default will suffice. I'm only cleaning up the ctor and dtor for now because I'll be completely redoing the class design anyway.

Oh, and while you do effectively have to prototype your class member functions (by writing the class body), I strongly recommend that instead of prototyping free functions at the top and defining them later, instead put them in order such that they only have to be prototyped where necessary (because of circular dependencies). By doing it that way, you expose said circular dependencies (because you will be forced to handle them) and save typing. C++ offers you this horrible one-pass compilation model; instead of fighting it with extra boilerplate, try to reap the few benefits. I also put the Player member function implementations next to the Player class body, so that they're all in one chunk (you should consider moving such things to a separate header/source file pair).

With that, plus my reformatting (you could at least be consistent about indentation amount), we get:

*** Source Snippet Removed ***

I'm out this evening, so more later; rest assured I will eventually fix the actual bug, but there's too much other stuff I have to comment on first, so that you actually learn something [wink]


Thanks for really trying to help me out.

As for the indentation, I was trying to fix the problem so I just kinda made sloppy code. [sad]

Well, please, help me out on any other design critizism, on STL info, anything you think I should know. I didnt know a vector created memory on the heap for you. Thanx for the tip.

Believe it or not, with all the mess I call my code, I fixed the bug [smile]

Once again, thank you. [smile]

Share this post


Link to post
Share on other sites
Hehe :) Well then, onwards...

Pass 2: This one is a big of a grab-bag. I'm cleaning up some unused things and silly algorithms. For this pass I've put the comments on what has been done in-line with the code. I've also attempted some of the separation of responsibility that is sadly missing in the original version.


#include <ctime>
#include <string>
#include <vector>
#include <iostream>
#include <algorithm>
#include <iterator>

using namespace std;

class Player {
public:
Player(const string& TheWord) : m_SoFar(TheWord.size(), '-'), m_numGuesses(0) {}

// Player::SetWord() wasn't implemented or used, so I killed the declaration.

// The Player class shouldn't be doing the printing internally! Things with names like
// GetSomething() should, well, *get something*, not display it.
// Anyway, we shouldn't be restricting ourselves to cout like that. Properly designed classes
// "present" themselves on a supplied ostream. But more on that later...

// So, for example, GetUsed() will return a string representation of the m_Used().
// This is inefficient, but the properly designed solution - which avoids the inefficiency -
// doesn't use an accessor at all; this is just a stepping stone...
string GetUsed() {
// I'll show a slick trick with algorithms and a stringstream...
stringstream ss(" "); // put in the leading space

// Now, append everything, with " " as a separator.
copy(m_Used.begin(), m_Used.end(),
ostream_iterator<char>(ss, " "));

return ss.str();
}

// The 'const' prevents other classes from using this for evil purposes.
// If we were to provide the non-const version as well, the compiler would select which
// one to use according to whether or not the instantiated Player was declared as const.
// This is called "const overloading". However, we're not using that here - even non-const
// Players will not be modified by *this* function :)
const string& GetSoFar() const {
return m_SoFar;
}

int GetNumGuesses(const string& TheWord) {
return TheWord.size() - m_NumGuesses;
}

// Similarly, a raw mutator - something named SetSomething() - should accept the setting
// value, and do something internal with it.
void SetGuess(char c) {
m_Guess = c;
}

// Instead of using reference parameters to return multiple booleans, why not acknowledge
// that the game state is something more complex than that?
typedef enum { WON, LOST, IN_PROGRESS } Result;
Result CheckWin(const string& TheWord);
void CheckGuess(const string& TheWord);

// No reason to have this be static when nothing else is...
// It's not as though, if you were to create multiple Players, they
// should have the same guess count. That also introduces the bug
// that the guess count wouldn't get reset the second time playing.
int m_NumGuesses;

private:
char m_Guess;
vector<char> m_Used;
string m_SoFar;
};

//------------------------------------------------------------------------------
// Name: CheckWin()
// Desc: checks to see if user has won
//------------------------------------------------------------------------------
Player::Result Player::CheckWin(const string& TheWord) {
if (m_SoFar == TheWord) {
return WON;
}

// Notice how, now that the accessor is written properly, we can actually *reuse it*.
if (GetNumGuesses(TheWord) <= 0) {
// By the way, you used to report a win in this case rather than a loss.
return LOST;
}

return IN_PROGRESS; // otherwise.
}

//------------------------------------------------------------------------------
// Name: CheckGuess()
// Desc: Checks to see if the users guess is correct
//------------------------------------------------------------------------------

// We'll need a helper struct...
struct CharMatcher() {
char guess;
CharMatcher(char guess) : guess(guess) {}

char operator()(char inWord, char known) {
// If the character actually in the word matches our guess, yield it;
// otherwise, yield the character that we already "know" - which is '-' for
// unguessed positions.
return (inWord == guess) ? inWord : known;
}
};

void Player::CheckGuess(const string& TheWord) {
// As written, the code tried to .find() on the string,
// for each character position, starting at the beginning each time. That's
// confused (and confusing), to say the least.
// Instead, what we should do is check if the letter is there once at the beginning:
// if so, we fill it in everywhere that it appears - using a standard library algorithm.

if (TheWord.find(m_Guess) == string::npos) {
// Not there, so increment the failure count and bail out.
// I *could* do the whole function as if/else-if, but this illustrates
// a very useful pattern known as a "guard clause". Don't be fooled by
// SESE advocates; doing things this way is idiomatic and avoids the "arrow anti-pattern".
m_NumGuesses++;
return;
}

// Otherwise: time for another std::algorithm :)
std::transform(TheWord.begin(), TheWord.end(),
m_SoFar.begin(), m_SoFar.begin(),
CharMatcher(m_Guess));
}

// END OF PLAYER CLASS

//------------------------------------------------------------------------------
// Name: Menu()
// Desc: Gets the user's desired action
//------------------------------------------------------------------------------
int Menu() {
system("CLS");

int menu = 0;
cout << "Welcome to Hangman\n"
<< "Programed by Brandon Wall\n\n"
<< "Menu Items:\n\n"
<< "0 = Quit\n"
<< "1 = New Game\n\n"
<< "Menu Selection: ";
cin >> menu;

system("CLS");

return menu;
}

//------------------------------------------------------------------------------
// Name: Pause()
// Desc: Waits for the user to continue
//------------------------------------------------------------------------------
void Pause() {
cout << "\nPress [ENTER] to continue.";
cin.ignore();
cin.get();
}

// Based on stuff I noted before, the naming here was wrong. So:
string GetWord() {
// There's no reason to shuffle the whole list to just pick one thing.
// Instead, we can just pick a random position. We don't have to do the
// hard part ourselves; there's a standard library algorithm for this, too.
// By the way, don't seed the RNG here; do it in main().

// Actually, re-creating the vector in here every time is silly. We should (a)
// create it just once and hold on to it somewhere; and (b) read it from a file.
vector<string> words;
words.push_back("business");
words.push_back("computer");
words.push_back("evergreen");
words.push_back("designer");
words.push_back("dynamite");

string result;
// Here I use a string* as an iterator over strings
// (although there just happens to be only one of them).
random_sample_n(words.begin(), words.end(), &result, 1);

return result;
}

void Play(const string& TheWord) {
Player::Result status = IN_PROGRESS;
Player player(TheWord);

while (status == IN_PROGRESS) {
system("CLS");
// Now we need to do the printing outside of those functions, because they no longer
// do printing that they shouldn't do. But now we're getting a better view of the
// code structure...
cout << "\nUsed Letters: " << player.GetUsed() << endl;
cout << "\nAmount of Guesses left: " << player.GetNumGuesses(TheWord) << endl;
cout << "\nThe Word So Far: " << player.GetSoFar() << '\n' << endl;

cout << "\nPlease enter a letter guess: ";
char guess;
cin >> guess;
player.SetGuess(guess);

player.CheckGuess(TheWord);
status = player.CheckWin(TheWord);
}
// Once we get here, the status is either WON or LOST.
// Notice the separation of responsibility: the CheckWin() function
// *checks* if the player won; it doesn't *report* the fact.
// Also notice the removal of duplication here ;)
// (In reality, this may be a step backwards in the real world, where you have to
// worry about things like internationalization. But in the real world, you have more
// advanced systems set up for printing these messages in a localized way anyway.
cout << "\n\nYou " << ((status == WON) ? "Won" : "Lost")
<< "! The word was " << TheWord << endl;
}

int main() {
srand(time(0));

bool done = false;
while (!done) {
// The proper scope for TheWord is in here.
// Except we can actually scope it more tightly, and don't
// even need the variable here at all...
switch (Menu()) {
case 0:
cout << "\n\nBye!\n";
Pause();
done = true;
break;

case 1:
// I've factored out this loop to keep things tidy. Notice how TheWord is eliminated.
// (Ok, ok, it isn't really eliminated; it just turns into a parameter...)
Play(GetWord());
break;

default:
cout << "\n\nInvalid Option.\n\n";
Pause();
}
}
// No need to return 0 explicitly...
}

Share this post


Link to post
Share on other sites
Pass 3: Now that the silly stuff is out of the way, we can rework the interface to the Player, and as a result, rename it. ;) Keep reading...

Let's look at how the class is currently used:


while (status == IN_PROGRESS) {
system("CLS");
cout << "\nUsed Letters: " << player.GetUsed() << endl;
cout << "\nAmount of Guesses left: " << player.GetNumGuesses(TheWord) << endl;
cout << "\nThe Word So Far: " << player.GetSoFar() << '\n' << endl;

cout << "\nPlease enter a letter guess: ";
char guess;
cin >> guess;
player.SetGuess(guess);

player.CheckGuess(TheWord);
status = player.CheckWin(TheWord);
}


The first three function calls are logically grouped together: there isn't really any reason we'd want to access those values individually - we'll always get all of them, in this order, with this formatting. This is a design assumption that could change later, but remember to KISS. Anyway, there are better solutions for adding flexibility here when it becomes needed: typically, one creates "PlayerRenderer" classes which either have friend access to the relevant information, or receive it in parameters.

So. What is the task this group of function performs? Can we give it a name? I think we can; what it's doing is *displaying* the game state. As I mentioned in the comments before, properly designed classes don't bind to std::cout, but instead display to a provided ostream, for reasons of flexibility. How should we spell the name of this function? In other languages, it might be "print" or "display" or "present" or even "render"; but to keep things simple, and C++-idiomatic [wink], I am going to spell it "operator<<" - with the expected interface for that operator.

Similarly, the second set of three function calls are grouped. More to the point, they're doing things in a rather round-about way: SetGuess() reads a value and store it into a data member, *just* so that CheckGuess() can read that member and use it. The data member isn't really a meaningful part of the object state; it only communicates between these functions.

If these were free functions, would you arrange the communication in the equivalent way - by using a global? Of course not: you'd use parameter passing, by having CheckGuess() accept a char, and having SetGuess() return a value that is then passed to CheckGuess(). But then, SetGuess() becomes trivial, and we end up eliminating it entirely. We end up with just CheckGuess(char) and CheckWin(char) - yeah, they both need that value, which is why storing it in a member almost made sense. But since we're going to fold those functions together into the common task they perform anyway... :)

So what is this task? We accept a character and use it to update the game state. That state is a combination of letters known so far, and whether or not we've won/lost yet. We could call this "update" (which emphasises the changes to the game state) or "process" (which emphasises the argument as a thing being processed), or perhaps by some other name to emphasise the yielded game status result. I'm going to make a decision that some might disagree with, and spell it "operator()". After all, this is the only "action" the Player can take now; display is "passive". Our object becomes something with one specific thing that it *does* (something stronger than having only one "responsibility", the goal indicated by the SRP), so it has something of the nature of a function.

Meanwhile, in order to make all of this work - while we remove the m_Guess as a member (it was just used for communication between functions that's no longer needed), we'll need to store TheWord as a member. This makes sense, after all: it's part of the abstraction that we're really trying to create here, as evidenced by the fact that we constantly need to pass it in to all the member functions.

Hmm... what *is* that abstraction, after all? I don't think it's really the Player I've been hating on, but... the Game. :D It tracks, updates and presents the current status of the game in progress, but more to the point, it does *not* have anything to do with the player's name, statistics on wins/losses, or anything else like that. It represents a single attempt at playing the Game of Hangman.

Armed with all of that design knowledge, we can redesign things much better. While I'm at it, I'm making the m_numGuesses private (we don't need public access to it after all).


#include <ctime>
#include <string>
#include <vector>
#include <iostream>
#include <algorithm>
#include <iterator>

using namespace std;

class Game {
public:
Game(const string& TheWord) :
m_Word(TheWord), m_SoFar(TheWord.size(), '-'), m_numGuesses(0) {}

friend ostream& operator<<(ostream& os, const Game& g);

typedef enum { WON, LOST, IN_PROGRESS } Result;
// I'll toss the helper struct into the Game class now;
// main() doesn't need to know it exists
struct CharMatcher() {
char guess;
CharMatcher(char guess) : guess(guess) {}

char operator()(char inWord, char known) {
return (inWord == guess) ? inWord : known;
}
};

Result operator()(char guess);

private:
// Used to be GetNumGuesses(), which was a bad name, upon reflection.
int remainingGuesses() {
return m_Word.size() - m_NumGuesses;
}

int m_NumGuesses;
string m_Word;
string m_SoFar;
vector<char> m_Used;
};

ostream& operator<<(ostream& os, const Game& g) {
// Don't flush the buffer within operator<< overloads;
// output '\n' instead of endl where appropriate. This
// is part of playing nice with the calling code, which
// normally doesn't expect buffer flushes in the middle
// of outputting individual variables.

os << "\nUsed Letters: ";
// Do the old GetUsed() work, outputting directly to os instead of
// a temporary stringstream - this is the efficiency tip I hinted at earlier
copy(g.m_Used.begin(), g.m_Used.end(), ostream_iterator<char>(os, " "));

return os << "\n\nAmount of Guesses left: " << g.remainingGuesses()
<< "\n\nThe Word So Far: " << g.m_SoFar << "\n\n";
}

Game::Result Game::operator()(char guess) {
// Update m_SoFar...
if (m_Word.find(guess) == string::npos) {
// Now I can't use the guard clause approach any more, because we still have to
// check the status.

// Oops, I clipped this part before somehow...
m_Used.push_back(guess);
m_NumGuesses++;
} else {
std::transform(m_Word.begin(), m_Word.end(),
m_SoFar.begin(), m_SoFar.begin(),
CharMatcher(guess));
}

// Then yield the resulting game status.
// I'm not writing this part like guard clauses this time, because it's at the
// end of the function rather than the beginning. Although you could, and I might
// in a different mood.
if (m_SoFar == m_Word) {
return WON;
} else if (remainingGuesses() <= 0) {
return LOST;
} else {
return IN_PROGRESS;
}
}

// END OF PLAYER CLASS

int Menu() {
system("CLS");

int menu = 0;
cout << "Welcome to Hangman\n"
<< "Programed by Brandon Wall\n\n"
<< "Menu Items:\n\n"
<< "0 = Quit\n"
<< "1 = New Game\n\n"
<< "Menu Selection: ";
cin >> menu;

system("CLS");

return menu;
}

void Pause() {
cout << "\nPress [ENTER] to continue.";
cin.ignore();
cin.get();
}

string GetWord() {
vector<string> words;
words.push_back("business");
words.push_back("computer");
words.push_back("evergreen");
words.push_back("designer");
words.push_back("dynamite");

string result;
random_sample_n(words.begin(), words.end(), &result, 1);

return result;
}

void Play(const string& TheWord) {
Game g(TheWord);
char guess;
// Look how slick this is now. ;)
Game::Result status = IN_PROGRESS;
while (status == IN_PROGRESS) {
system("CLS");
// Note that the "Please enter a letter guess: " phrase doesn't logically belong
// as part of the representation of the Game, so I output it separately.
cout << g << "\nPlease enter a letter guess: ";
char guess;
cin >> guess;
status = g(guess);
}

cout << "\n\nYou " << ((status == WON) ? "Won" : "Lost")
<< "! The word was " << TheWord << endl;
}

int main() {
srand(time(0));

bool done = false;
while (!done) {
switch (Menu()) {
case 0:
cout << "\n\nBye!\n";
Pause();
done = true;
break;

case 1:
Play(GetWord());
break;

default:
cout << "\n\nInvalid Option.\n\n";
Pause();
}
}
}



Pass 4 (at least) to come...

Share this post


Link to post
Share on other sites
Quote:
Original post by NUCLEAR RABBIT
As for the indentation, I was trying to fix the problem so I just kinda made sloppy code. [sad]

BTW, if you are using visual studio, yuou can select the whole code (or just portions of it) and hit Alt*F8 - it will automatically indent your code accordingly to what is defined in the editor options for the selected language.

Regards,

Share this post


Link to post
Share on other sites
Nevar!!!11

Pass 4. ^^

Ok, this is the last one. I could plug in a menu system, but at this point it doesn't really accomplish anything. See comments for details; there's not too much to say this time - if you understood this far, the rest should be pretty easy to follow.


#include <ctime>
#include <string>
#include <vector>
#include <iostream>
#include <algorithm>
#include <iterator>
#include <sstream> // yay, one more.

using namespace std;

// I made a few simplifications in the game class... keep reading and compare
// to the previous version...
class Game {
public:
Game(const string& TheWord) :
m_Word(TheWord), m_SoFar(TheWord.size(), '-'), m_numGuesses(0) {}

friend ostream& operator<<(ostream& os, const Game& g);

typedef enum { WON, LOST, IN_PROGRESS } Result;

struct CharMatcher() {
// So that we can do everything in one pass, I'm also going to record some
// state in here: whether or not any match was found. You have to be
// careful with "stateful predicates" because the order in which the
// matcher is applied technically isn't guaranteed, but we only care about
// whether *any* inWord matched the guess.
bool anyMatch;
char guess;
CharMatcher(char guess) : guess(guess), anyMatch(false) {}

char operator()(char inWord, char known) {
bool match = inWord == guess;
if (match) { anyMatch = true; }
return match ? inWord : known;
}
};

Result operator()(char guess);

private:
// Actually, we don't need m_NumGuesses, because it's just a count of
// the used characters. We already have that: m_Used.size(). :)
int remainingGuesses() {
return m_Word.size() - m_Used.size();
}

string m_Word;
string m_SoFar;
vector<char> m_Used;
};

ostream& operator<<(ostream& os, const Game& g) {
os << "\nUsed Letters: ";

copy(g.m_Used.begin(), g.m_Used.end(), ostream_iterator<char>(os, " "));

return os << "\n\nAmount of Guesses left: " << g.remainingGuesses()
<< "\n\nThe Word So Far: " << g.m_SoFar << "\n\n";
}

Game::Result Game::operator()(char guess) {
// Now we can use our modified predicate to check the guess in one pass.
CharMatcher cm(guess);
std::transform(m_Word.begin(), m_Word.end(),
m_SoFar.begin(), m_SoFar.begin(),
CharMatcher(guess));

if (!cm.anyMatch) { // No match.
m_Used.push_back(guess);
}

// Then yield the resulting game status.
if (m_SoFar == m_Word) {
return WON;
} else if (remainingGuesses() <= 0) {
return LOST;
} else {
return IN_PROGRESS;
}
}

// Next, we introduce the new Dictionary class.
class Dictionary {
vector<string> words;

public:
Dictionary(const string& filename) {
// Read all words from the file into our collection.
ifstream is(filename.c_str());
string s;
while (is >> s) { words.push_back(s); }
// We can do that with std::copy, too, but I thought I'd show a common
// file-reading idiom. :)
}

// Now that we have the file data stored somewhere, we can load it just
// once, and pick a word each time without reloading:
string operator()() {
string result;
random_sample_n(words.begin(), words.end(), &result, 1);
return result;
}
};

int Menu() {
system("CLS");

int menu = -1;
// we want it to trigger the default case if a value isn't read
cout << "Welcome to Hangman\n"
<< "Programed by Brandon Wall\n\n"
<< "Menu Items:\n\n"
<< "0 = Quit\n"
<< "1 = New Game\n\n"
<< "Menu Selection: ";
input(cin, menu);

system("CLS");

return menu;
}

void Pause() {
cout << "\nPress [ENTER] to continue.";
cin.sync();
char c;
input(cin, c);
}

void Play(const string& TheWord) {
Game g(TheWord);
char guess;
// Look how slick this is now. ;)
Game::Result status = IN_PROGRESS;
while (status == IN_PROGRESS) {
system("CLS");
cout << g << "\nPlease enter a letter guess: ";
char guess;
input(cin, guess);
status = g(guess);
}

cout << "\n\nYou " << ((status == WON) ? "Won" : "Lost")
<< "! The word was " << TheWord << endl;
}

// Here's the utility function we will use to make input more robust.
template <typename T>
bool input(istream& is, T& result) {
string line;
getline(is, line);
return stringstream(line) >> result;
// If the input was garbage, 'result' is untouched, and false is returned.
// Otherwise, 'result' contains the new value, and true is returned.
// In our case, the return value won't get used, but it's useful in the
// general case.
}

template <>
bool input(istream& is, string& result) {
return getline(is, line);
}

int main() {
srand(time(0));

Dictionary d("hangman.txt");

bool done = false;
while (!done) {
switch (Menu()) {
case 0:
cout << "\n\nBye!\n";
Pause();
done = true;
break;

case 1:
Play(d());
break;

default:
cout << "\n\nInvalid Option.\n\n";
Pause();
}
}
}

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this