Help me optimize my code - Beginner Guessing Game

[marcoakis]

Hi guys, I solved this C++ problem for my mid semester assignment. It is a guessing game where the user will guess a number between 1 and 10 and will bet and win/lose money accordingly.

I just started programing with C++ so I'd like to receive feedback on how to optimize/polish my code.

#include <iostream>
#include <iomanip>
#include <ctime>
using namespace std;

int main()
{
//declare variables
float balance = 100; //initial balance
float bet; //bet value 
int guessNumber; //The number user guess is an integer (1-10)
int computerNumber; //The number computer randomly generate is an integer (1-10)
float loseDollars; //the amout of dollars to lose when giving a wrong guess
//welcome message
cout << "CP1200 Guessing Game" << endl;
cout << "Written by " << endl << endl; 
//formatted in the standard way with two decimal places
cout << fixed << setprecision(2);
//input the bet
cout << "Please enter your bet (up to $" << balance << "): $";
cin >> bet;
srand( time(0) ); // set the seed of random variables
//main guessing and betting procedure
while( (bet != 0) && (balance > 0))
{
// check the validity of betting
if( (bet > balance) || (bet < 0) )
{
bet = balance;
cout << "Your bet is $" << bet << endl;
}
cout << endl << "Guess a number between 1 and 10: ";
cin >> guessNumber;
computerNumber=1 + rand() % 10; // generate a random value from 1 to 10
// correct guess
if( guessNumber == computerNumber )
{
cout << endl << "Correct!" << endl;
cout << "You win $" << bet << endl<<endl;
balance = balance + bet; 
}
//wrong guess
else 
{
cout << endl << "Wrong! The computer chose: " << computerNumber << endl;
loseDollars = bet / 5.0 * abs( guessNumber - computerNumber );
if (loseDollars >= balance)
{
cout << "You lose $" << balance << endl << endl;
balance = balance - loseDollars;
}
else
{
cout << "You lose $" << loseDollars << endl << endl;
balance = balance - loseDollars;
}
}

// check the balance after guess
if( balance > 0 )
{
cout << "Your new balance is $" << balance << endl << endl;
cout << "Please enter your bet (up to $" << balance << "): $";
cin >> bet;
}

}//end while
//farewell message
cout <<endl<< "Thank you for playing!" << endl;
system("pause");
return 0;
} //end of main function

[Ancient Dragon]

The first place to start is to improve the program's formatting. Writing the code with everyting starting on the left margin is not only ugly but very difficult to read. You might get more responses by improving the program's formatting.

[marcoakis]

The first place to start is to improve the program's formatting. Writing the code with everyting starting on the left margin is not only ugly but very difficult to read. You might get more responses by improving the program's formatting.

I corrected the formating and edited a few things, this is the lastest code.

Once again, the program works, but I'd like you input on how to optimize make the code more elegant.

Thanks!

// A1.cpp
// Written by XXX XXXX
// Data finished: XXXX XXXX 
// Program description: 

#include <iostream>
#include <iomanip>
#include <ctime>

using namespace std;

int main()
{
	//Variables
	float bal = 100; //initial balance
	float bet; //bet value 
	int guessNumber; //The number user guess is an integer (1-10)
	int computerNumber; //The number computer randomly generate is an integer (1-10)
	float lostDollars; //the ammout of dollars to lose when giving a wrong guess

	//Welcome message
	cout << "CP1200 Guessing Game" << endl;
	cout << "Written by XXX XXX " << endl << endl; 

	//Setting up 2 decimal places
	cout << fixed << setprecision(2);

	//Calling srand() function

	
	//Prompt and user bet

	cout << "Please enter your bet (up to $" << bal << "): $";
	cin >> bet;
	srand( time(0) ); 

	//Loop until user has some money or a bet of 0 is entered
	while( (bet != 0) && (bal > 0))
	{
	if( (bet > bal) || (bet < 0) )
		{
			bet = bal;
			cout << "Your bet is $" << bet << endl;
	
	//Asking for guess and generating random number 
		}
		cout << endl << "Guess a number between 1 and 10: ";
		cin >> guessNumber;
		computerNumber=1 + rand() % 10; 

		//User won and giving balance update
	if( guessNumber == computerNumber )
		{
		cout << endl << "Correct!" << endl;
		cout << "You win $" << bet << endl<<endl;
		bal = bal + bet; 
		}

	//User lost
	else 
		{
		cout << endl << "Wrong! The computer chose: " << computerNumber << endl;
		lostDollars = bet / 5.0 * abs( guessNumber - computerNumber );
	
		//Calculating the lost amount
		if (lostDollars >= bal)
		{
		cout << "You lose $" << bal << endl << endl;
		bal -= lostDollars;
		}
	else
		{
		cout << "You lose $" << lostDollars << endl << endl;
		bal -= lostDollars;
		}
}

	//Updating user balance
	if( bal > 0 )
	{
	cout << "Your new balance is $" << bal << endl << endl;
	cout << "Please enter your bet (up to $" << bal << "): $";
	cin >> bet;
}

}
	//End loop

//Farewell message
	cout <<endl<< "Thank you for playing!" << endl;
	
	system("pause");
	return 0;

}

[WaltP]

You should concentrate a little more on your formatting. Indenting 8 spaces is too many for most situations. And your indentation is a little inconsistent.

See this for some ideas. There are many other suggestions on the web.

[marcoakis]

You should concentrate a little more on your formatting. Indenting 8 spaces is too many for most situations. And your indentation is a little inconsistent.

See this for some ideas. There are many other suggestions on the web.

Thanks WaltP. I reviewed the indentation and fixed it a bit.... bear with me, I've only been learning C++ for 4 weeks.

Updated code:

// DelgadoMarcoA1.cpp
// Written by Marco Delgado
// Data finished: May 6th 2010
// Program description: 

#include <iostream>
#include <iomanip>
#include <ctime>

using namespace std;

int main()
{
	//Variables
	float bal = 100; //initial balance
	float bet; //bet value 
	int guessNumber; //The number user guess is an integer (1-10)
	int computerNumber; //The number computer randomly generate is an integer (1-10)
	float lostDollars; //the ammout of dollars to lose when giving a wrong guess

	//Welcome message
	cout << "CP1200 Guessing Game" << endl;
	cout << "Written by Marco Delgado" << endl << endl; 

	//Setting up 2 decimal places
	cout << fixed << setprecision(2);
	
	//Prompt and user bet

	cout << "Please enter your bet (up to $" << bal << "): $";
	cin >> bet;
	//Calling srand() function
	srand( time(0) ); 

	//Loop until user has some money or a bet of 0 is entered
	while( (bet != 0) && (bal > 0))
	{
	if( (bet > bal) || (bet < 0) )
		{
			bet = bal;
			cout << "Your bet is $" << bet << endl;
		}
	//Asking for guess and generating random number 
		
		cout << endl << "Guess a number between 1 and 10: ";
		cin >> guessNumber;
		computerNumber=1 + rand() % 10; 

		//User won and giving balance update
	if( guessNumber == computerNumber )
	{
		{
		cout << endl << "Correct!" << endl;
		cout << "You win $" << bet << endl<<endl;
		bal = bal + bet; 
		}
	}
	//User lost
	else 
	{
		cout << endl << "Wrong! The computer chose: " << computerNumber << endl;
		lostDollars = bet / 5.0 * abs( guessNumber - computerNumber );
	
		//Calculating the lost amount
		if (lostDollars >= bal)
		{
		cout << "You lose $" << bal << endl << endl;
		bal -= lostDollars;
		}
	else
		{
		cout << "You lose $" << lostDollars << endl << endl;
		bal -= lostDollars;
		}
	}
	//Updating user balance
	if( bal > 0 )
	{
	cout << "Your new balance is $" << bal << endl << endl;
	cout << "Please enter your bet (up to $" << bal << "): $";
	cin >> bet;
	}
}
	//End loop
//Farewell message
	cout <<endl<< "Thank you for playing!" << endl;
	
	system("pause");
	return 0;

}

[WaltP]

It's still bad. I added a link for a reason.

[VernonDozier]

Visual C++ Format option to the rescue, plus changing all tabs to 4 spaces. On Daniweb, a tab is 8 spaces, so if you mix tabs and spaces it looks terrible.

// DelgadoMarcoA1.cpp
// Written by Marco Delgado
// Data finished: May 6th 2010
// Program description: 

#include <iostream>
#include <iomanip>
#include <ctime>

using namespace std;

int main()
{
    //Variables
    float bal = 100; //initial balance
    float bet; //bet value 
    int guessNumber; //The number user guess is an integer (1-10)
    int computerNumber; //The number computer randomly generate is an integer (1-10)
    float lostDollars; //the ammout of dollars to lose when giving a wrong guess

    //Welcome message
    cout << "CP1200 Guessing Game" << endl;
    cout << "Written by Marco Delgado" << endl << endl; 

    //Setting up 2 decimal places
    cout << fixed << setprecision(2);

    //Prompt and user bet

    cout << "Please enter your bet (up to $" << bal << "): $";
    cin >> bet;
    //Calling srand() function
    srand( time(0) ); 

    //Loop until user has some money or a bet of 0 is entered
    while( (bet != 0) && (bal > 0))
    {
        if( (bet > bal) || (bet < 0) )
        {
            bet = bal;
            cout << "Your bet is $" << bet << endl;
        }
        //Asking for guess and generating random number 

        cout << endl << "Guess a number between 1 and 10: ";
        cin >> guessNumber;
        computerNumber=1 + rand() % 10; 

        //User won and giving balance update
        if( guessNumber == computerNumber )
        {
            {
                cout << endl << "Correct!" << endl;
                cout << "You win $" << bet << endl<<endl;
                bal = bal + bet; 
            }
        }
        //User lost
        else 
        {
            cout << endl << "Wrong! The computer chose: " << computerNumber << endl;
            lostDollars = bet / 5.0 * abs( guessNumber - computerNumber );

            //Calculating the lost amount
            if (lostDollars >= bal)
            {
                cout << "You lose $" << bal << endl << endl;
                bal -= lostDollars;
            }
            else
            {
                cout << "You lose $" << lostDollars << endl << endl;
                bal -= lostDollars;
            }
        }
        //Updating user balance
        if( bal > 0 )
        {
            cout << "Your new balance is $" << bal << endl << endl;
            cout << "Please enter your bet (up to $" << bal << "): $";
            cin >> bet;
        }
    }
    //End loop
    //Farewell message
    cout <<endl<< "Thank you for playing!" << endl;

    system("pause");
    return 0;
}

[marcoakis]

Thanks guys,

I reviewed the code following WaltP's link and I cleaned up the code.

What do you think?

// DelgadoMarcoA1.cpp
// Written by Marco Delgado
// Data finished: May 6th 2010
// Program description: 

#include <iostream>
#include <iomanip>
#include <ctime>

using namespace std;

int main()
{
    //Variables
    int ranNum;
    int userNum;
    double bal = 100;
    double bet;
    double lostDollars;

    //Welcome message
    cout << "CP1200 Guessing Game" << endl;
    cout << "Written by Marco Delgado" << endl << endl;

    //Setting up 2 decimal places
    cout << fixed << setprecision(2);

    //Prompt and user bet

    cout << "Please enter your bet (up to $" << bal << "): $";
    cin >> bet;
    //Calling srand() function
    srand(time(0));

    //Loop until user has some money or a bet of 0 is entered
    while ((bet != 0) && (bal > 0)) {
	if ((bet > bal) || (bet < 0)) {
	    bet = bal;
	    cout << "Your bet is $" << bet << endl;
	}
	//Asking for guess and generating random number 

	cout << endl << "Guess a number between 1 and 10: ";
	cin >> userNum;
	ranNum = 1 + rand() % 10;

	//User won and giving balance update
	if (userNum == ranNum) {
	    {
		cout << endl << "Correct!" << endl;
		cout << "You win $" << bet << endl << endl;
		bal = bal + bet;
	    }
	}
	//User lost
	else {
	    cout << endl << "Wrong! The computer chose: " << ranNum << endl;
	    lostDollars = bet / 5.0 * abs(userNum - ranNum);

	    //Calculating the lost amount
	    if (lostDollars >= bal) {
		cout << "You lose $" << bal << endl << endl;
		bal -= lostDollars;
	    } else {
		cout << "You lose $" << lostDollars << endl << endl;
		bal -= lostDollars;
	    }
	}
	//Updating user balance
	if (bal > 0) {
	    cout << "Your new balance is $" << bal << endl << endl;
	    cout << "Please enter your bet (up to $" << bal << "): $";
	    cin >> bet;
	}
    }
    //End loop
//Farewell message
    cout << endl << "Thank you for playing!" << endl;

    system("pause");
    return 0;

}

[VernonDozier]

The program is good. Ways it could be better would be if you told the user how to quit (enter 0) and if you told them they couldn't play anymore after they had no money. Also, you check whether they have enough money to cover their bet when they lose. It would be better to check at bet time. Also, how about asking how much the person wants to bet each game rather than only once in the beginning?

[WaltP]

Much better! I personally prefer the { on a separate line (like Vernon's code) but that's a preference. I just find code easier to follow that way.

It also alleviates this problem:

if (userNum == ranNum) {
	    {
		cout << endl << "Correct!" << endl;
		cout << "You win $" << bet << endl << endl;
		bal = bal + bet;
	    }
	}

[marcoakis]

The program is good. Ways it could be better would be if you told the user how to quit (enter 0) and if you told them they couldn't play anymore after they had no money. Also, you check whether they have enough money to cover their bet when they lose. It would be better to check at bet time. Also, how about asking how much the person wants to bet each game rather than only once in the beginning?

I can't really change the specifications of the program since my deliverable must be exactly what they are requiring.

When you talk about changing the money validation before the user lose, can you give an example of it?

On your last point, the program prompts for the betting amount every time you guess a number.

[VernonDozier]

I can't really change the specifications of the program since my deliverable must be exactly what they are requiring.

When you talk about changing the money validation before the user lose, can you give an example of it?

On your last point, the program prompts for the betting amount every time you guess a number.

You're right on the last point. I didn't notice that at the end. I saw the bet adjusting at the top and the checking for whether you had enough to cover the bet, but no chance to change a bet that you couldn't cover and just assumed. My bad.

Validation would be something like this. Instead of automatically changing a bad bet to a good bet, give an error message and prompt them again.

bool goodBet = false;
while (!goodBet)
{
    cout << "Enter bet : ";
    cin >> bet;
    if (bet <= bal)
        goodBet = true;
    else
        cout << "You don't have that much money.  Try again.\n";
}