Jump to content
  • Advertisement

Archived

This topic is now archived and is closed to further replies.

RedRabbit

Optimize my code please :)

This topic is 5225 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hey guys somebody a while back told me the best way to begin programming games as an amatuer to both game programming and programming in itself is to start with guess the number, then work on tetris, and on up from there. I made a guess the number application and It works half decent. Ive got the concept down (whether it be acceptable in the eyes of the programming community or not) i think. Please anybody give suggestions/optimizations to my code and most important please explain what youve done . Thanks a bunch!
#include <iostream>
#include <time.h>

using namespace std;

int main()
{
	int Number;
	srand((unsigned)time(NULL));
	int Answer = rand()%101;
	cout << "Guess a number from 1-100: ";
	cin >> Number;
	if(Number != Answer)
	{
		while(Number > Answer)
		{
			cout << "Too high, guess again: ";
			cin >> Number;
		}
		while(Number < Answer)
		{
			cout << "Too low, guess again: ";
			cin >> Number;
		}
	}
	if(Number == Answer)
	{
		cout << "You win!" << endl;
		cout << "Quitting...";
		char Choice;
		cin >> Choice;
		return 0;
	}
	return 0;
}

Share this post


Link to post
Share on other sites
Advertisement
It doesn''t appear to allow the user to "guess again". You need to put the whole thing in a while(!done){} type loop.

Share this post


Link to post
Share on other sites
That program''s too simple to have to optimize. It''s a 20-line console program, I doubt it takes up that much CPU time that you need to optimize it. For future reference, only try to optimize a program if it lags on you. Even though, use a profiler to check what''s causing the lag.

Share this post


Link to post
Share on other sites
Additionally, rand()%101 will yield a value between 0-100. But yeah I''d have to agree that this code couldn''t possibly be optimized because of its size/lack of repetetive function (such as if one line was repeated 3427213 times.

Share this post


Link to post
Share on other sites
Perhaps optimize wasnt the right terminology sorry, I actually was hoping somebody could rewrite or writeover the *noobness* and put my thoughts into accepted coding, if that makes any sense :-\. Like the style, I know there is a better way to code it and thats sort of what Im asking for.

Share this post


Link to post
Share on other sites
I just rewritten your program:

#include <iostream>
#include <time.h>
using namespace std;

int main(){
int Number = 1 ;
int Answer ;

while (Number > 0) {
srand((unsigned)time(NULL));
Answer = rand()%101;

cout << "Guess a number from 1-100: ";
cin >> Number;

while (1) {
if (Number > Answer)
cout << "Too high, guess again: ";
else if (Number < Answer)
cout << "Too low, guess again: ";
else {
cout << "You win!\n" ;
break ;
}

cin >> Number;
}

cout << "Enter > 0 to play again, < 0 to quit:" ;
cin >> Number ;
}
return 0;
}

Share this post


Link to post
Share on other sites
A few suggestions:
- Use more comments, particularly when you do non-obvious things (like that pause input at the end).
- Standard C headers in C++ are named differently than their pure C counterparts. (ie: stdlib.h should be cstdlib, stdio.h should be cstdio).
- I like to avoid "using namespace" statements, but that is somewhat of a personal preference thing.
- Your rand statement includes 0 which, according to your text, is a bug.
- Organize your code into nuggets of work, where each chunk of code does a specific job. Don't let it mush together and become spagetti-like.

Here's how I would write it. Only thing I don't like about it is how I used two input statements, one inside, and one just before, the while loop. That could probably be done better.

// Number guessing game.  User must guess a random number that the computer

// picks. When the user is wrong, the computer gives them a hint about the

// relative magnitude of the number.


#include <iostream>
#include <cstdlib>

int main()
{
unsigned int guess;
unsigned int answer(std::rand() % 100 + 1);

// The number guessing loop. Continues to ask the user for the number

// until they get it right.

std::cout << "Guess a number between 1 and 100:";
std::cin >> guess;
while (guess != answer)
{
if (guess > answer)
{
std::cout << "Too high - guess again.";
}
else
{
std::cout << "Too low - guess again.";
}
std::cin >> guess;
}

// User wins! Jubilant crowds fill the streets.

std::cout << "You win the prize!";

// Wait for a keypress before quitting.

char pause;
std::cout << "Press any key to quit...";
std::cin >> pause;

return 0;
}


Edit: Fixed tags, etc.

[edited by - dcosborn on May 31, 2004 12:04:17 AM]

Share this post


Link to post
Share on other sites
Your logic is screwy. Right now you have this:


if(Number != Answer)
{
while(Number > Answer)
{
...
}
while(Number < Answer)
{
...
}
}
if(Number == Answer)
{
...
}

Depending on what combination of high/low numbers the user enters, the program behaves completely differently, and in some cases just terminates before the user even guesses the answer. What you really want is this logic:

while(Number != Answer)
{
if(Number > Answer)
{
...
}
if(Number < Answer)
{
...
}
}
// User has answer down here

See the difference? You''re loop logic is now based on whether or not the user guesses the answer correctly, while inner conditional logic determines if the guess is too high or too low.

Share this post


Link to post
Share on other sites
Beaten, so I'll post my version anyway

#include <ctime>
#include <iostream>
int main()
{
bool answerFound = false;
int number;
srand((unsigned)time(NULL));
int answer = rand()%101;


while(answer != number) {
std::cout << "Guess a number from 1-100: ";
std::cin >> number;
if(number < answer) { std::cout << "Too low, guess again: "; }
else if (number > answer) { std::cout << "Too high, guess again: "; }

std::cout << "You win!" << std::endl;
system("PAUSE");
return 0;
}


[edited by - xMcBaiNx on May 31, 2004 11:59:40 PM]

Share this post


Link to post
Share on other sites

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!