Hi I am currently in school right now as a senior in college with a major of Electrical Enginnering getting ready to graduate in Dec. I know some programming but it comes more on the hardware side like microcontrollers interatcing with sensors and things like that with GPIO pins etc.. so nothing too crazy. But I have always liked games and like coding classes I have taken. Dont get me wrong I love my major but I also like the progamming aspect. The only class I have taken was a class in C and then another class of continuation into C++. So I have started small and made a basic text based guess the number game with different functions such as saving scores to a file with a name, an exiting function, display all the scores and delete all the score. What I am looking for is some advice on structure and good coding pratices thing that I should do better on and some basic advice. Any advice would be greatly appericated.

#include <iostream>
#include <string>
#include <fstream>


using namespace std;	

int play();				// play the game
void save(int score);	// save score in a file
int menu();				// display a menu
void deleteData();		// delete the high scores
void displayScores();	// display the scores

int main()
{
	int status;			// returned from function
	int score = 0;		// how many times you have won
	int menuState = -1;	// menu state
	bool game;			// game state of game

	cout << "Get a score of 5 and win but three wrong guesses in a row and you lose\n";
	
	

	while (menuState != 2)
	{
		menuState = menu();
		game = true;
		if(menuState == 1)
		{
			while(game == true)
			{
				status = play();
				if (status == 1)
				{
					score += status;
					cout << "Score: " << score << "\n\n";
				
				}
				if (status == -1)
				{
					cout << "Sorry you are a loser!!!\n";
					cout << "Your final score was: " << score << " points\n";
					save(score);
					score = 0;
					menuState = -1;
					game = false;
								
				}
				if (score == 5)
				{
					cout << "Congrats you have won the Game!!\n";
					cout << "Your final score was: " << score << " points\n";
					save(score);
					score = 0;
					menuState = -1;
					game = false;
				}
			}			
		}
		if(menuState == 3)
		{
			displayScores();
		}
		if(menuState == 4)
		{
			deleteData();
		}

	}
	
	cout << "Exiting Game....\n";
	system("PAUSE");

	return 0;
}

int play()
{
	int randomNumber = rand() % 100 + 1;
	int tries = 0;
	int guess;
	

	while (tries < 3)
	{
		cout << "Guess a number between 1 and 100: ";
		cin >> guess;
		if (guess == randomNumber)
		{
			cout << "Congrats you guessed correctly\n";
			return 1;
		}
		else if (guess < randomNumber)
		{
			cout << "You guessed too low try again\n";
			tries++;
		}
		else if (guess > randomNumber)
		{
			cout << "You guessed too high try again\n";
			tries++;
		}
	}
	cout << "The number was " << randomNumber << "\n";
	return -1;
}

void save(int score)
{
	string name;
	cout << "Please enter you name: ";
	cin >> name;

	ofstream file;
	file.open("scores.txt",ios::app);
	if(file.is_open())
	{
		file << name << " " << score << "\n";
	}
	else
	{
		cout << "Unable to open file!!\n";
	}

	file.close();
}

int menu()
{
	int selection = -1;
	while(selection < 1 || selection > 3)
	{
		cout << "Make a selection\n";
		cout << "1 - Play Game\n";
		cout << "2 - Exit Game\n";
		cout << "3 - Display High Scores\n";
		cout << "4 - Erase High Scores\n";
		cout << "Input: ";
		cin >> selection;
		if(selection == 1)
			return 1;
		else if(selection == 2)
			return 2;
		else if(selection == 3)
			return 3;
		else if(selection == 4)
			return 4;
		else
			cout << "Please enter a valid selection!\n";
	}
	

}

void deleteData()
{
	ofstream file;

	file.open("scores.txt",ios::out | ios::trunc);
	file.close();
}

void displayScores()
{
	string name;
	int score;
	ifstream file;
	
	file.open("scores.txt");
	if(file.is_open())
	{
		while(true)
		{
			if(!(file >> name)) break;
			cout << name << "\t";
			if(!(file >> score)) break;
			cout << score << "\n";
		}
	}
	else
	{
		cout << "File could not be opened..\n";
	}
	
	file.close();
}

You should read up on how to structure a game loop.
You can then restructure and simplify your code, removing those inner loops and instead remember in which state you are.
Its roughly that:

enum State state=start;
string inputdata;
while(state!=stop) {
    draw(&state,inputdata);
    inputdata=input(&state);
}

You could use some better names. Good names make code so much clearer and it's one of the defining lines that separates good programmers from bad (or new) ones. Take some time to name your artifacts.

bool playingGame = true; // Instead of just the variable "game"
 
void deleteData(); // Deletes the high scores.
// vs.
void deleteHighScores();  // Having a comment say this Deletes high scores is redundant, unnecessary, and redundant.

The use of "status" is a bit wonky and the comment "returned from a function" doesn't help at all. After looking at your code it looks like play() returns 1 on success and -1 on failure. A better name for this might be playResult, and instead of returning magic numbers, you could return an enumeration.

// Declare this guy at the top of your file (or in a separate header file that you include)
enum PlayResult
{
    Lost = -1,
    Won = 1,
}
 
// Have play() return a PlayResult instead of an int.
PlayResult play();
 
// Then your code where you call it will look something like this
 
playResult = play();
if(playResult == PlayResult.Won)
{
    // score += status; // Adding your "status" to your score was the really wonky bit
    score += 1; 
}
else if(playResult == PlayResult.Lost)
{
    // Lost code here
}

Enumerations are great for getting rid of lots of "magic numbers" - numbers that are hardcoded all over the place whose meaning is obfuscated without digging deeper into the code. Looking at the line: if(status == 1) compared to if(playResult == PlayResult.Won), you can see how enumerations convey more information. Enumerations are C++ 11 features I think? If you don't have C++ 11 , you can do something similar with defined constants or a struct with static constant members.

Other good candidates for enumerations are your menuState variables.

It looks like you have some identical blocks of code. Any time you can copy/paste a block of code, you're probably better off writing a function for it. But each function should only do one thing.

...
if(playResult == PlayResult.Lost)
{
    endGame("Sorry you are a loser!!!\n");
}
if(score == 5) // Another magic number that could be replaced with a constant.
{
    endGame("Congrats you have won the Game!!\n");
}
 
// Since your game state variables were local to main, we'll have to wrap them up into a GameState struct (not shown) and pass them by reference into this new endGame function.
void endGame(string gameOverMessage, GameState &gameState)
{
    displayScore(); // function does: cout << "Your final score was: " << score << " points\n";
    cout << gameOverMessage;
    save(score);
    resetGameState(gameState);
    /*  function does:
        score = 0;
        menuState = -1;
        game = false;
    */
}

For such a simple program, splitting that block into a function that calls three separate functions is probably overkill, but you asked for best practices. The short version is DRY (Don't Repeat Yourself) - wrap identical code blocks into a function. And make sure a function does one thing (single responsibility principle). If you don't do this, then the next time someone comes through and needs to mess with the end game logic, they might go to the "you lost" section and make their changes there and not realize that they also needed to make those changes in the "you won 5 games" section. Then they just introduced a bug.

This is much less of an issue in a 200 line program, but when you're working on a big team in a project with millions of lines of code, it's a problem.

All in all, I think you did a good job. I haven't tested your code, but from reading it, it looks like it should work, and there was only that one bit of wonkiness with the status (that also still worked).

- Eck

the longest program i've written was 50k lines

programming style... i much prefer single character variables whenever possible, usually make it to 2 and 3 character variable names after a few thousand lines.

i consider my code to be concise and readable. other people hate it.

i do occasionally use a long variable name if it is only going to be used once or twice for an obscure function, but the meat and potatoes is all single or double character names.

never let anyone rate you. you'll see that people have so many ways to write code that aspiring for a unified convention is best done if you source from your own individual preferences.

and there are still some grown-ups in the world who think it's alright if you have those.

people may slag my unabashed advocacy of doing things the way that makes the most sense to you, but they're probably not typing by hanging over the end of a bed because they can't bend their legs. i've been typing on computer keyboards since 1981 and i've got no time to type out longvariablenameseventeen all day.

programming style... i much prefer single character variables whenever possible, usually make it to 2 and 3 character variable names after a few thousand lines.

i consider my code to be concise and readable. other people hate it.

If you run into an interviewer like me while trying to find a job, this is going to prevent us from hiring you.

If you only ever work by yourself, then you are free to code however you want. However, if you only work by yourself, then your conventions don't matter to anyone else, so why bother mentioning them to anyone else? You apparently have time to type out posts, but not time to use longer variable names? Don't make me laugh.

i much prefer single character variables whenever possible, usually make it to 2 and 3 character variable names after a few thousand lines.

i consider my code to be concise and readable. other people hate it.

(Emphasis mine)

These two goals are, in my opinion, completely contradictory.

Readable to me means that I can look at a variable, function, class or whatever, and not have to guess at what it means.

GIven context, I'll probably be able to figure out whether at means animationTime or angryTeddy, but that's an unneeded mental burden. However light that burden might be, it will add up and cause confusion at some point.

That doesn't mean I'm advocating naming things long just for the sake of being descriptive -- especially when it related to math and math formulae, short or even single-character variable names can make a lot of sense. If you're doing wave math, c would be valid instead of speedOfLight, in my eyes.

That said, typing out long names all the time can be annoying. But that's also why newer IDE's and editors have various features for alleviating that concern. IntelliSense, code completion, etc. Typing in a couple of characters should be enough to reduce the number of possible names to a just a couple, easily selected.

I also slightly disagree with the "never let anyone rate you" paragraph. I think I see where you're coming from; that optimal workflow is an individual thing, and one shouldn't be "forced" to conform for the sake of conforming.

That said, especially for a beginner, asking as many questions as possible can be a very positive thing. Getting answers and learning good habits before bad ones are too deeply ingrained, get critique on something that's recently completed and is fresh in the mind, etc.

In addition to what has already been said I would suggest...

1. As a general guideline you want to separate your code into more discreet pieces so a function does one thing only. Code is cleaner, easier to maintain/understand and opens the door to unit testing.
2. You are returning integers to represent states and it was suggested to use enumerations which is great. However, for error conditions I would suggest exploring exceptions instead.
3. Read up on classes (OOP) at some point. Nibble on it in small pieces. I would start with data hiding (private, public and protected variables and functions). This will keep you from stepping over yourself when you get more ambitious. It seems simple now but if you hit a time crunch at 2am you are more likely to experience less pain and frustration.

notice that i posted my opinion instead of rating your post down. the mistake so many people make that is the REASON I POST OPINION CONTRARY TO POPULAR is that many people seem to feel that there is only ONE envaluation in productivity, the "apparent commercial standard".

YES it is necessary to standardise when working on a team, but this is not everyone's objective. if someone wants to get hired straight out of school, then kowtow.

if on the other hand someone engages in a rich and varied set of activities during their adult/professional life, more "localised" methods are more appropriate, with a lower need for standardisation and readability.

the point you should understand is that short variable names are not a function of skill or duration of experience, it is a function of not being part of a standardised procedure.

if you think someone who is used to conventions established with c is a bad or inexperienced programmer, you're "self-inflated" and "other depreciating". the reason why you find it "ironic" is because you need to feel as if you have outwirtted me, instead of simply acknowledging that the methods of others may also be entirely valid.

if i'm writing a game, it's likely to be under 2000 lines total. for the scope of such a project, there is absolutely no need for it.

and, so you know, i don't have time to type out superfluous appellations,but i have all the time in the world to explain myself and encourage you to be less depreciative toward others. i am not bad, nor am i new. it's that simple.


i am not bad, nor am i new.

and, i do not use long variable names.

and that is really all that needs to be said, or considered.

now go ahead and nuke me for asserting that you shouldn't depreciate this expression.

blalocka2012 specifically asked for "good programming practices". While 1-2 character variable names might work just fine for your own personal projects, it's definitely not considered a good practice.

To show you why it's not a good practice, tell me what this code means:

fd = bd * wm - max(ta - ap, 0);
fd = max(fd, 0);

You could tell me that the code multiplies bd by wm and subtracts (ta-ap) when the result is greater than zero. Then it sets fd to zero if the result is negative. But that's what the code does... What does the code mean? If you have to read two dozen lines of surrounding code just to get a context, the code isn't readable.

When I was learning C++ in college (about a billion years ago), I took pride in my ability to keep complex programs straight in my head while only using 1-2 characters for variable names. But I was wrong. I was wrong about thinking it was a good idea, and I was wrong to think that my college programs were complex. :) In nearly every programming job, you're going to be working on a team. You're going to want to use good names for your own sake and the sake of your teammates.

- Eck

Thanks guy I appericate all the feedback and input. I think ill keep this program and try to make those modifications to it before I go one then I will try to do something like tic-tac-toe in console then move onto trying to replicate in in a graphics format since the logic should still stay the same then go from there.