#include <time.h>
#include <iostream>
using namespace std;
int main()
{
int a;
int b;
srand(time(NULL));
a = rand() % 21;
cout << " GUESS THE NUMBER GAME!\n";
cout << " an unoriginal idea coded by Kegan Walsh\n\n\n\n";
while (a!=b)
{
cout << "Guess a number between 1 and 20.\n";
cout << "> ";
cin >> b;
cout << "\n";
if (a>b)
{
cout << "Too low\n\n";
}
if (a<b)
{
cout << "Too high\n\n";
}
if (a==b)
{
cout << "That's right!\n\n";
system("Pause");
}
if (b<1)
{
cout << "Error: That number is below the given range.\n\n";
}
if (b>20)
{
cout << "Error: That number exceeds the given range.\n\n";
}
}
}
My Guess the Number Code
Author
Can you guys check it and see if there's anything I could've done better, or if my code structure is sloppy? Thanks!
The rand, srand, and time things were all new to me. It was kind of just copied from a tutorial. I sort of got what the tutorial was saying, but I have a C++ book coming in tomorrow, so I'll completely learn it then ;)
EDIT*: This may be nit-picky, but I think I'm going to make the "Too Low" and "Too High" into "Too Low." and "Too High." (added periods). So, if that's the only thing you'd correct about it, I already have it covered, so no need to post ;)
try using if else statments, for instance both statments if(a<b) if (b>a) can not be true at the same time so why check them both
So if(a<b) if(b>a) will turn into if (a<b) else if (b>a) and try putting the rare cases first like the if (b < 1) so it doesn't check all of the statment if clearly the answer is wrong and skip the rest of the checks
So if(a<b) if(b>a) will turn into if (a<b) else if (b>a) and try putting the rare cases first like the if (b < 1) so it doesn't check all of the statment if clearly the answer is wrong and skip the rest of the checks
It would be good practice to initialise b to something before checking its value. As your code stands, there is a small (incredibly small, but still present) chance that b will equal a before the user gets to type anything. This is because variables are not auto-initialised in C/C++ - they contain whatever random value was in their particular piece of memory, so it's a good idea to initialise variables when you declare them, unless you know they're going to be set to something before you ever use them. Try initialising b to a + 1 to ensure that they're not equal when the comparison is made in the while loop.
The following are just my suggestions:
Other then the above, it looks good to me. Nice work![smile]
Quote:Original post by scjohnno
It would be good practice to initialise b to something before checking its value. As your code stands, there is a small (incredibly small, but still present) chance that b will equal a before the user gets to type anything. This is because variables are not auto-initialised in C/C++ - they contain whatever random value was in their particular piece of memory, so it's a good idea to initialise variables when you declare them, unless you know they're going to be set to something before you ever use them. Try initialising b to a + 1 to ensure that they're not equal when the comparison is made in the while loop.
All true, but a better statement of your intent in this code would be to use a read-ahead loop. Bung the input into a function so you don't have to repeat it, then:
int get_number(){ int x; std::cout << "Give us a number: "; std::cin >> x; return x;}int main(){ int a=std::rand()%21; int b=get_number(); while(a!=b) { // do your stuff b=get_number(); }}Not knocking scjohnno's solution, which would certainly work, but a read-ahead loop would better document the intent of the code in this case.
Quote:Original post by KeganWalsh
Can you guys check it and see if there's anything I could've done better, or if my code structure is sloppy? Thanks!
The rand, srand, and time things were all new to me. It was kind of just copied from a tutorial. I sort of got what the tutorial was saying, but I have a C++ book coming in tomorrow, so I'll completely learn it then ;)
*** Source Snippet Removed ***
EDIT*: This may be nit-picky, but I think I'm going to make the "Too Low" and "Too High" into "Too Low." and "Too High." (added periods). So, if that's the only thing you'd correct about it, I already have it covered, so no need to post ;)
Aside from what was already mentioned, there are a couple of little things. First, you are not doing any input checking so it is possible to break your program. While this may not be important for such a simple little program, it would be a good idea to get into the habit of checking for the validity of input.
Give your variables more meaningful names. 'a' and 'b' don't mean much.
Use a do...while loop. In this situation it would make more sense because you should not be checking the condition until the user has provided some input. This also solves the initialization problem, and in this case you wouldn't have to initialize the variable as no matter what happens the user would provide input before it is accessed. It might still be a good idea to get into the habit of initializing variables regardless.
Your random number generator can set a to 0 in this case. That could be a problem since you don't allow your player to select 0. Your random number setup should be something like:
a = (rand() % 20); // remainders of anything divided by 20 are 0 to 19a += 1; // make the range 1 to 20Last but not least it is theoretically possible that your console out buffers are not flushed to the screen. It is a good idea to get into the habit of using std::endl or std::flush prior to asking for user input to ensure they can see what it is you're asking for. In this case, you would do the following:
cout << "Guess a number between 1 and 20.\n";cout << "> " << flush;cin >> b;In the case of this code, the use of cin may automatically flush the buffers, however this is another good habit to get in to just in case it is necessary.
Also, just to exercise your abilities: you could implement a method for allowing the player to play the game again, as well as a counter for how many tries it took the player to guess the number. That way you can tell them whether they did well or not :)
Quote:Original post by Crypter
The following are just my suggestions:You should use ctime over time.h. time.h is an old C header, not C++. I personally advise against using namespace std for alot of reasons. Get into the habit of initializing all variables as soon as they are declared. C++ does not do this for you, so all of your variables contain unknown values. I personally recommend against using system(). It is not very portable. I second the suggestion to use if..else statements.
Other then the above, it looks good to me. Nice work![smile]
About system:
Considering it's a console application in the first place, it doesn't really matter if it's not portable. He's probably not going to send it to anyone, and if he does, they'll most likely be on windows anyways. The rest, I do agree with though.
Author
Thanks for all of the suggestions guys! My book came in yesterday (It's now 12:01 AM, so Oct. 18 heh) after school, so I'll read through it, use the old C++ workshops (I'm using the same book), and learn new techniques. I'll keep all of these comments in my memory.
Thanks again!
Thanks again!
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
