Hmm... Something wrong while building. brb
You do not include <string> anywhere, so Game.cpp can't build.
/********* main.cpp **********/
#include <iostream> //not used
#include "Game.h"
using namespace std; //not used - also sometimes not recommended
int main()
{
Game game;
game.initialize();
}
/**************** Game.h *****************/
#ifndef GAME_H_INCLUDED
#define GAME_H_INCLUDED
#include <string>
using namespace std; //do not 'using namespace' in the global scope of a header file: it forces the namespace on everything that includes the header.
class Game{
private:
string playerName; //you do not #include <string> in this header. (see notes after code)
int amountOfGuesses;
void initializeDifficulty(int difficulty); //maybe "setDifficulty()"?
public:
void initialize(); //constructors are for initialization
void startGame(); //this name is not correctly descriptive
};
#endif // GAME_H_INCLUDED
// ^ good work on commenting this to match the opening clause
/**************** Game.cpp *****************/
#include <cstdlib>
#include <iostream>
#include "Game.h"
void Game::initializeDifficulty(int difficulty)
{
while(amountOfGuesses == 0){ //what? why is this a loop? More importantly, why would the var be zero!?
if(difficulty == 1) //use a 'switch()' here
amountOfGuesses = 9;
if(difficulty == 2) //why isn't this 'else if()' ?
amountOfGuesses = 6;
if(difficulty == 3)
amountOfGuesses = 3;
if(difficulty != 1 && difficulty != 2 && difficulty != 3)
cout << "Invalid input, please try again." << endl; //this does not belong in this function
}
}
void Game::initialize()
{
int userInput;
cout << "Hello there, let's play a number guessing game!" << endl << "What's your name?" << endl << "Name: "; //see notes after code on use of std::endl
cin >> playerName;
cout << "Hello, " << playerName << " do you want to play Easy (1), Normal (2) or Hard (3)?" << endl;
cin >> userInput; //you need to sanitize input
initializeDifficulty(userInput);
startGame(); //does this belong here?
}
void Game::startGame()
{
int userInput;
int timesGuessed = 0;
int randomNumber; //this could stand to have a better name - name should describe purpose rather than origin
do{
randomNumber = rand() % 25 + 1; //minor point - see notes on rand()
cout << "Okey let's start " << playerName
<< " you need to guess a number between 1 and 25." << endl
<< "You may guess " << amountOfGuesses << " times max. Good luck!" << endl << endl;
while(amountOfGuesses - timesGuessed > 0) { //inconsistent brace style
cout << "You have still " << amountOfGuesses - timesGuessed << " chances left." << endl;
cout << "Enter a number: " << endl;
cin >> userInput; //sanitize input
if(userInput == randomNumber)
break;
else{
timesGuessed++;
if(userInput > randomNumber)
cout << "That's too high try again" << endl;
if(userInput < randomNumber) //'else if'
cout << "That's too low try again" << endl;
}
}
if(userInput == randomNumber)
cout << "Good job, you guessed my number in " << timesGuessed + 1 << " times" << endl;
if(userInput != randomNumber) //'else if'
cout << "You didn't guess my number in time. My number was " << randomNumber << endl;
cout << "Do you want to play again (1) or quit (000)" << endl;
cin >> userInput; //sanitize input
} while(userInput != 000); //why '000'? (see notes after code)
}
Beginning with main.cpp:
You include <iostream> but do not make use of it. Probably this is because you were having trouble with Game.h, where you declare a std::string but don't include <string>. You state 'using namespace std', but there is no use of the std namespace here.
Game.h
You declare a std::string here, but don't include <string>. This can be a gotcha in VS because <iostream> prototypes std::string but does not define it. The result is that Game.cpp won't compile because there are missing operator overloads. I added the <string> inclusion to Game.h to make the project compile.
initializeDifficulty() is simply setting the values associated with difficulty. It may be more descriptive to call it 'setDifficulty()'.
Constructors are used to initialize classes. An initialize() member function is always a red flag. (see notes in Game.cpp section below)
startGame() is here, but it actually runs the whole game rather than starting it.
Game.cpp
Game::initialize() is not initializing the Game object. It's gathering user input and doing miscellaneous work, then calling the function that runs the game. It also does not sanitize its input, which I'll explain later. For now, you should probably rename this function to something like "get player data", and remove the call to startGame().
Game::initializeDifficulty() has several problems. For now let's correct the use of what's here:
- Why on earth is there a while() loop here? If the input is bad this creates an infinite loop of failure. If the input is good then the loop simply does not repeat. More importantly, if the value of amountOfGuesses is not zero to start with (protip: it's not zero) then this loop will NEVER RUN!
- You're printing out an error message to the user here, but this function is not intended for user interaction. The function should attempt to set the relevant value, and if it fails then it should return an error to the calling function. The calling function interacts with the user, so print the error message from there.
- You have a series of 'if()' statements that are logically exclusive. Use 'else if()' for this. In this instance it doesn't cause an error, but it does cause needless work. In slightly more complicated cases it can cause all kinds of errors.
- Research the use of the 'switch()' statement.
Game::startGame() uses a modulated rand() call. This application is trivial, so it's not a big deal, but modulating on rand() does not result in an even distribution. More importantly, you forgot to seed the randomizer, so it's likely that the number is going to be the same every time the program runs.
[attachment=26738:494c867093.png]
There's an excellent lecture on this by Stephan Lavavej here: http://channel9.msdn.com/Events/GoingNative/2013/rand-Considered-Harmful
Your brace style is inconsistent. You use hanging braces for functions and inline braces in other places. You may want to pick one and stick with it. (Personally I hate hanging braces, but they appear to be getting more popular.)
At the end of your loop you check for (userInput != 000). Do not add zeroes to things where they are not needed. This changes the type of the numeric value. In this case the value is the same, but if you had said 008, for instance, you'd have gotten a compiler error because a number starting with an extra zero is interpreted as a value in octal. Also, boolean tests in C and C++ accept zero as false and non-zero as true. You can just say "while(userInput)", though in this case it makes sense to compare to zero explicitly because you're using zero as a token value from the user.
General Issues:
You use std::endl all over the place. That doesn't just insert a line-break. It also flushes the stream. When you're just printing out a boat-load of text, use the "\n" escape character to insert a new-line. Use std::endl when you are done with output and you want the buffer to flush before moving on to the next line.
You do not sanitize your user input! I can get this program to go crazy at almost any point by simply providing a string in a place where a number is expected. Try it out.
std::cin is kind of a pain in the butt in this case. Here's a function I use to get numbers from cin:
template<typename T>
T getUserValue(string requestString) {
T retval;
while(true) {
cout << requestString;
cin >> retval;
bool ok = cin.good();
cin.clear();
cin.ignore(numeric_limits<streamsize>::max(), '\n');
if(ok) {break;}
cout << "Invalid response.\n";
}
return retval;
}
//usage:
int result = getUserValue<int>("Enter an integer: ");
float otherResult = getUserValue<float>("Enter a float: ");
See if you can sort out everything that's going on there and understand the pitfalls of std::cin and how this function deals with them.
Once the technical problems are corrected, let us know and we can talk about the overall program structure a little bit.
