Hangman Game - Feedback?

[Evo303]

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

#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;
}

[Paul McKenzie]

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.

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.

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

Regards,

Paul McKenzie

[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.

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

[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.

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

#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

[Gonzo45]

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

[Evo303]

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.