Hangman Game - Feedback?

Hi guys,

I've been lurking around for a while but, having now learned the basics of C++, i thought i'd try and make a simple Hangman game using a text file for the list of possible words...

So, basically, if you are bored and have a spare 10 minutes, would any of you kind gentlemen like to review my code and suggest areas where i can improve?

I greatly appreciate ANY and all criticism, it's the best way to learn, even if you are mean to me ;D

Code file + dictionary also attached

Thanks in advance!

P.S. Ignore the unnecessary headers - I was testing my memory ;P

Code:

---

#include <iostream>
#include <cmath>
#include <string>
#include <time.h>
#include <fstream>
#include <cstring>
#include <cctype>

using namespace std;

//Prototypes
void print(int, char *p);
bool check_letter(int, char, char *p, int *result);


int main()
{
 
 //Limit for incorrect guesses
 const int limit = 10;
 char wrongguesses[11] = {'0','0','0','0','0','0','0','0','0','0'};
 
 bool game_over = false;
 bool game_won = false;
 int attempts = 0;
 char actualword[50];
 char guessedword[50];
 string word;
 int lines = 0;
 int r = 0;
 int i = 0;
 int icount = 0;
 char guess;
 int position[10] = {-1,-1,-1,-1,-1,-1,-1,-1,-1,-1};
 bool foundletter = false;
 
 srand(time(NULL)); //Seed the random number generator with system time
 
 ifstream file("Words.txt"); //Open the words file
 
 if (!file.is_open()) //Check if file could not be opened
 {
  cout << "Dictionary file could not be opened!" << endl;
  system("pause");
  return 0;
 }
 
 //Count lines in the file
  while (getline(file,word))
 {
      lines++;
 }
 
 file.clear();  //Start again from the start of the file
 file.seekg(0);
 
 r = rand() % lines; //Randomise a line number to get a word from
 
 //Go to the random line number and extract the word
  while (i != r)
 {
      getline(file,word);
      i++;
 }
   
    strcpy (actualword, word.c_str()); //Copy the STRING into the CHARACTER ARRAY for use below
   
    int wordlength = strlen(actualword);
   
    //Fill the array with blanks of the correct length + spaces
    for (int i = 0; i < wordlength; i++)
    {
        if (actualword[i] == ' ')
          guessedword[i] = ' ';         
        else
          guessedword[i] = '_';
        }
   
    //Print output
    print(wordlength, guessedword);
   
   
   
    ///////////////////////////////////////////////////////
    //Begin game loop
    ///////////////////////////////////////////////////////
   
    while (attempts < limit && !game_won)
    {
         
          //List the incorrect attempts
          cout << endl << endl << "Incorrect Letters: ";
                 
          for (int i = 0; i < attempts; i++)
          {
              cout << wrongguesses[i] << " ";
          }
             
          cout << endl << endl << "Enter a letter [" << limit - attempts << " incorrect guesses remaining]: " ;
          cin >> guess;
         
          guess = tolower(guess);
                 
          //Check for the letter in the actual word
                 
          foundletter = check_letter(wordlength, guess, actualword, position);
                 
                             
                  for (int i = 0; i < 10; i++)
                  {
                    if (position[i] != -1)
                      guessedword[position[i]] = actualword[position[i]];
                  }
         
          //Check for underscores - if there are none then the word is complete (Game_Won)
          icount = 0;
                   
          for (int i = 0; i < wordlength; i++)
          {
              if (guessedword[i] == '_')
                icount++;
              }
         
          if (icount == 0)
            game_won = true;
         
          //Incorrect Letter         
          if (!foundletter)         
            {

            //Check for the letter - if found then don't punish again
            for (int i = 0; i < limit; i++)
                {
                if (wrongguesses[i] == guess)
                    {
                    goto nextguess;
                    }
                }
            //Add the incorrect letter to the incorrect array + increase count 
           
            wrongguesses[attempts] = guess;
            attempts++;
            nextguess:;
            }
           
            print(wordlength, guessedword);
           
    }
   
    ///////////////////////////////////////////////////////
    //End game loop
    ///////////////////////////////////////////////////////
   
   
   
    //Win or Lose
   
    if (game_won)
      {
      cout << endl << endl << "Cogratulations! You guessed the word with only " << attempts << " incorrect guesses!" << endl;
      }
    else
      {
          cout << endl << endl << "Game Over! Word: " << actualword << endl;
      }
         
       
    system("pause");
    return 0;
}

void print(int len, char *p)
{
    system ("cls"); //Clear the screen
    cout << len << " characters" << endl << endl;
    for (int i = 0; i < len; i++)
    {
        cout << *(p + i) << " ";
        } 
}

bool check_letter(int len, char c, char *p, int *result)
{   
    bool found = false;
    int x = 0;
    for (int i = 0; i < len; i++)
    {
        if (*(p + i) == c || *(p + i) == toupper(c))
        {
            result[x] = i;
            x++;
            found = true;
          }
    }
    return found;
}

---

Code:

---

char actualword[50];
char guessedword[50];
string word;

---

You should use string for all of the string variables and get rid of the char arrays. Then you don't need things like this.

Code:

---

strcpy (actualword, word.c_str()); //Copy the STRING into the CHARACTER ARRAY for use below

---

Beginner or simple C++ programs that have things like strcpy() sprinkled in them IMO is a flawed program. This not only wouldn't be necessary, it is a hole in your program if word.c_str() is > 50 characters. Even if there are probably no words that have > 50 characters, you should get into the habit of using strings all the time, and only if there is a compelling reason to do so, you use arrays.

Code:

---

string actualword;
string guessedword;
string word;
//...
actualword = word; // no chance of buffer overrun.

---

Regards,

Paul McKenzie

Hi Paul,

Thanks very much for taking the time to help me out :]

The reason i chose to use character arrays for the actual and guessed word is so that i could run through them and check / change single characters by just using a for loop to loop through the elements in the array.

If i used strings for these 2 variables, what would be the best way of changing or checking individual letters in the string? and how could i refer to the character directly?

Thanks again

Quote:

---

Originally Posted by Evo303

Hi Paul,

Thanks very much for taking the time to help me out :]

The reason i chose to use character arrays for the actual and guessed word is so that i could run through them and check / change single characters by just using a for loop to loop through the elements in the array.

---

You can do the same thing with string. If you couldn't, then std::string would be almost useless.

Code:

---

#include <string>
//...
int main()
{
  std::string x = "abc123";
  x[0] = 'b';
}

---

The string variable can be used just like an array of char.

Regards,

Paul McKenzie

Haha I like this. Not bad for someone just learning the basics.

Cheers Gonzo ;D

Paul, thank you very much for your help, i've changed those character arrays to strings now and everything still works nicely :)

Thanks again for your help. :thumb: