### Rand() mis-generates numbers in the hundreds of thousands?

This program is a game. Two random numbers are generated, and the user must guess where the two numbers will match at. However, my rand() function generates numbers in the hundreds of thousands, when it should be 50-200.

Here's the code: (my rand function is at line 116)

 ``123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156`` ``````#include #include #include #include #include // ALWAYS include when using Sleep function; #include using namespace std; int first = 5; int second = 2; int switchit; int advancedfirst, advancedsecond; int guess; void firstnumber() // randomize first number { first = rand() % 200 + 50; } void secondnumber() // randomize second number { second = rand() % 200 + 50; } int advancedfirstnumber() { advancedfirst = rand() % 500 + 50; return advancedfirst; } int advancedsecondnumber() { advancedsecond = rand() % 500 + 50; return advancedsecond; } void advanced() { system("CLS"); //ask player to enter number guess; get number; create loop for 50-500 int advancedguess; cout<<"********************************************\n\n\t\t\tADVANCED - 00000065432\n\n********************************************\n\n"; cout<<"Welcome to Advanced difficulty. The number range has moved up from 50-200 to 50-500."; cout<<"I trust you have the patience for this one; it'll surely take a while.\n"; cout<<"\n\t\t\t\tShall we begin?:"; cout<<"\n\t\t\t"; string begin; getline(cin, begin); if (begin == "yes" || begin == "sure" || begin == "ok" || begin == "yeah" || begin == "yea" || begin == "hell yeah") { cout<<"\n\n\t\t\t\tVery well.\n"; } else { cout<<"\n\n\t\t\t\tWell hurry up, I'll give you 20 seconds to get ready.\n"; Sleep(5000); } advancedrestart: cout<<"\nEnter a number between 50 and 500: "; cin>>advancedguess; cout<<"\n\n"; do { // display number, two spaces, display second, two spaces advancedfirstnumber(); cout<< advancedfirstnumber(); cout<<" "; advancedsecondnumber(); cout<< advancedsecondnumber(); cout<<" "; } while (advancedfirst != advancedsecond); // check if the guess is equal to where the two numbers matched if (advancedguess == advancedfirst) { cout<<"\n\n\t\t\t\tVery nice.\n\n"; cout<<"Well that's it for now, I'll be adding in another level soon.\n"; Sleep(2000); exit (1); } else { cout<<"\n\n\t\t\t\tThat is incorrect."; Sleep(5000); cout<<"\n\n\t\t\t\t\t ..."; goto advancedrestart; } } int main() { restart: srand ( time(0) ); cout<<"\n\nIn this game, random numbers will be generated until those two numbers match.\n\n"; Sleep(4000); cout<<"You will guess which number the two will match at.\n\n"; Sleep(4000); cout<<"If you get it wrong, it will restart.\n"; cout<<"Get ready.\n\n"; Sleep(5000); cout<<"3.. "; Sleep(400); cout<<"2.. "; Sleep(400); cout<<"1.. "; Sleep(400); cout<<"go!\n\n\n"; // 3.. 2.. 1.. go! cout<<"\n\nGuess a number between 50 and 200: "; cin >> guess; while (first != second) { first = rand() % 50 + 200; cout<< first; cout<<" "; second = rand() % 50 + 200; cout<<" "; cout<< second; } if (guess == first) { if (guess == second) options: cout<<"Nice.\n\n\n\n\n\n\t\t\t\t"; cout<<"1/ Restart\n\t\t\t\t"; cout<<"2/ Move on to Advanced difficulty\n\t\t\t\t"; cout<<"3/ Exit.\n"; cin>>switchit; switch (switchit) { case 1: goto restart; break; case 2: advanced(); break; case 3: exit (1); break; default: cout<<"You had to enter 1-3, please do so.\n"; goto options; break; } } else if (guess != first) { first = 5; second = 2; goto restart; } }``````

Thanks for all your help, I really need it on this one.
Last edited on
Please avoid gotos. Also, you are using <ctime> and <time.h> the C and C++ versions of the same header. It seems you are using C++ so only use the <cname> files and not the <name.h> ones. I'm not sure if this has to do with it, but I think you ought to seed the random generator with srand(). Just looking at it I don't see why rand would do that, although I think you have your ranges messed up.

`rand()% range + min`
Holy crap I am amazed with this one. While running the program over and over to find the bug, I noticed that I had numbers like:

12380 89120 23190 2580

Do you see what all of those numbers have in common? I want all of you to see if you can find the bug from what I have told you in this post. If you can't figure it out, just say so and I'll tell you.

What do these numbers have in common about them?
closed account (z05DSL3A)
You would more likely see numbers like:
 ... Guess a number between 50 and 200: 111 247 201234 224230 215249 203200 220227 201241 214225 206209 201232 202235 219217 220226 235 200 245209 202233 241208 201233 226246 208219 206217 218242 213248 226225 211204 208232 244 244 233224 205230 248248 248 ...
Yes, your while loop does not print a space after the second value so you are printing

first secondfirst secondfirst

etc.

Also, your loop is not quite right anyway:

 ``123456789`` `````` do { // display number, two spaces, display second, two spaces advancedfirstnumber(); cout<< advancedfirstnumber(); cout<<" "; advancedsecondnumber(); cout<< advancedsecondnumber(); cout<<" "; } while (advancedfirst != advancedsecond);``````

Every time you call advancedfirstnumber(), it generates a different number. Likewise with advancedsecondnumber(). Why are you calling both functions twice?

both return the values they generate. firstnumber() and secondnumber() should do the same. None of those functions should be using global variables.
Congrats, jsmith.

The loop was fine, working as intended (after bug fix). It continuously makes the random numbers until they both match. For the global variables, I don't really see a flaw in it big enough to make them local. The program is small so it won't affect speed and performance heavily.

 ``123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191`` ``````#include #include #include #include // ALWAYS include when using Sleep function; #include using namespace std; int first = 5; int second = 2; int switchit; int advancedfirst, advancedsecond; int guess; int advcodetrue = 3; int firstnumber() // randomize first number { first = rand() % 150 + 50; return first; } int secondnumber() // randomize second number { second = rand() % 150 + 50; return second; } int advancedfirstnumber() { advancedfirst = rand() % 500 + 50; return advancedfirst; } int advancedsecondnumber() { advancedsecond = rand() % 500 + 50; return advancedsecond; } void advanced() { system("CLS"); //ask player to enter number guess; get number; create loop for 50-500 int advancedguess; cout<<"********************************************\n\n\t\t\tADVANCED - 00204065432\n\n********************************************\n\n"; cout<<"Welcome to Advanced difficulty. The number range has moved up from 50-200 to 50-500."; cout<<"I trust you have the patience for this one; it'll surely take a while.\n"; cout<<"\n\t\t\t\tShall we begin?:"; cout<<"\n\t\t\t"; string begin; getline(cin, begin); if (begin == "yes") { cout<<"\n\n\t\t\t\tVery well.\n"; } advancedrestart: cout<<"\nEnter a number between 50 and 500: "; cin>>advancedguess; cout<<"\n\n"; do { // display number, two spaces, display second, two spaces advancedfirstnumber(); cout<< advancedfirstnumber(); cout<<" "; advancedsecondnumber(); cout<< advancedsecondnumber(); cout<<" "; } while (advancedfirst != advancedsecond); // check if the guess is equal to where the two numbers matched if (advancedguess == advancedfirst) { cout<<"\n\n\t\t\t\tVery nice.\n\n"; cout<<"Well that's it for now, I'll be adding in another level soon.\n"; Sleep(2000); exit (1); } else { cout<<"\n\n\t\t\t\tThat is incorrect."; Sleep(3000); cout<<"\n\n\t\t\t\t\t ..."; goto advancedrestart; } } int main() { int levelskip; int advancedcode; cout<<"1.Continue\n2. Skip to Advanced\n"; cin >> levelskip; switch (levelskip) { case 1: break; case 2: cout<<"Code: "; cin >> advancedcode; if (advancedcode == advcodetrue) { advanced(); break; } else { cout<<"Incorrect code, continuing\n"; Sleep(1000); system("CLS"); break; } default: break; } srand ( time(NULL) ); cout<<"\n\nIn this game, random numbers will be generated until those two numbers match.\n\n"; Sleep(4000); cout<<"You will guess which number the two will match at.\n\n"; Sleep(4000); cout<<"If you get it wrong, it will restart.\n"; cout<<"Are you ready? "; string ready; getline(cin, ready); if (ready == "yes") { cout<<"Ok.\n"; } else { cout<<"Fine, you have 5 seconds.\n"; Sleep(5000); } cout<<"3.. "; Sleep(400); cout<<"2.. "; Sleep(400); cout<<"1.. "; Sleep(400); cout<<"go!\n\n\n"; // 3.. 2.. 1.. go! restart: cout<<"\n\nGuess a number between 50 and 200: "; cin >> guess; do { firstnumber(); cout<< firstnumber() << ", "; secondnumber(); cout<< secondnumber() << ", "; } while (first != second); if (guess == first) { if (guess == second) options: cout<<"\nNice.\n\n\n\n\n\n\t\t\t\t"; cout<<"1/ Restart\n\t\t\t\t"; cout<<"2/ Move on to Advanced difficulty\n\t\t\t\t"; cout<<"3/ Exit.\n"; cin>>switchit; switch (switchit) { case 1: first = 5; second = 2; goto restart; break; case 2: advanced(); break; case 3: exit (1); break; default: cout<<"You had to enter 1-3, please do so.\n"; goto options; break; } } else if (guess != first) { first = 5; second = 2; goto restart; } } ``````
As a matter of good programming practice, global variables are a last resort.

The problem with global variables is that it makes your program very difficult for other developers to understand. (And even the original programmer, after enough time has passed).

I believe the main problem with globals is namespace polution.
Yeah I didn't think of it that way. The thing with global variables is that they continue to exist after their use has been performed so it adds on to the size of the program, and any heavy program with lots of global variables are sure to cause a hefty lag increase
Warrior, the real problem with globals has nothing to do with performance. It is all about maintainability and understandability.

The definition of a correct programmer is one which generates the correct output for all possible inputs. (Not all inputs are valid). The only way to prove that a program is correct is to know the state of the program at all points for a given input or inputs. Because global variables can be modified absolutely anywhere in the program, any code that uses the global variables is a real strain to understand exactly what it does and what output it generates.

Consider the following examples.

 ``12345678910111213`` ``````// Example A int global_A; int multiply( int b ) { return global_A * b; } // Example B int multiply( int a, int b ) { return a * b; }``````

In example A, for any given input b, I cannot say what the output of the function will be unless I know the value of global_A. But global_A can be set anywhere in the million-line program that it is part of. The only way I know what multiply( 5 ) returns is if I can know what global_A was last set to. In a large program, this is generally very hard if not impossible.

In example B, I know what multiply( 4, 5 ) does. I know what multiply( some_var_x, some_var_y ) does only if I know the values of some_var_x and some_var_y.

C++ experts generally recommend that you minimize the scope of your variables only to what you need. Why? Well, two reasons. One is memory usage, though if we're talking simple ints and chars and such it will take a lot of them to make a difference. The other is to aid in the understanding of the program and proving correctness.

[Ok, there is a third reason, particularly when dealing with objects that need to be constructed with parameters, but I won't go into that.]
Topic archived. No new replies allowed.