Sign in to follow this  
soitsthateasy

My Newb Program Error...

Recommended Posts

someone please help me!!! the problem is so basic im nearly embarressed to show everyone...
#include <iostream>
#include <cstdlib>
#include <ctime>

int input, guess;


using namespace std;
int main()
{
int input;
cout<<"\t\tGUESS MY NUMBER";
cout<<"\nEnter a number between 1 and 100:";
cin>>input;
while(input>100 || input<0)
{
cout<<"\nPick a number between 1 and 100";
cin>>input;
}

{
    while(guess!=input)
    {
    int theGuess(int guess);
    cout<<guess;
    }
    if(guess==input)
    {
        cout<<"\n\nYour number was "<<guess;
        return 0;
    }
}
return 0;
}

int theGuess (int guess)
{
    if(guess!=input)
{
    srand(time(0));
    guess=rand()%100+1;
    return guess;
}
}

I'm trying to make a program that the user inputs a number and the computer guesses it. I also want to show the computers guesses... Problem 1: I can't generate a fully random number... Problem 2: I can't show the computer's guesses... Problem 3: The game just goes into an infinite loop when you run it because I can't generate a totally random number. Stupidly easy game, Stupidly easy fix??? Thanks in advance

Share this post


Link to post
Share on other sites
You are calling srand() every time before you generate a random number using rand(). srand() (re)initialises the random number generator using the given seed. In your case it should be called once, at the start of your program.
Although your seed is different almost every time you call srand(), the first number generated by rand() is usually very much alike. This usually leads programmers to generate one or more random numbers before actually using them to ensure the randomness is properly started.

Share this post


Link to post
Share on other sites
In addition to the srand() correction, you've got a lot of funky stuff going on. First, you should properly indent everything. That means everything inside of two curly brackets { ...this stuff... } should be indented one level more than the stuff outside the brackets. The contents of your main() function should all be indented one level. The contents of a while loop should be indented one further, etc. This greatly improves the readability of the code.

Next, if you look you've actually got two input variables declared. There's the one at the very top which is a global variable and can be accessed everywhere. And there's another at the top of main(). One problem is that the one in main() covers up the global one while the program is in main. That is, all references to input in main() will be referring to the local one declared at the top of main(), not the global one. The problem then comes up in theGuess(), where input is the global one not the local one. However, in the next paragraph I'll tell you why you don't need to use input at all in theGuess. In other words, you can just remove the global input entirely from your code.

Let's look next at your function theGuess(). First, it's name is a noun or a thing. Functions generally refer to actions, so you generally want some sort of verb or action in the name. For example, "makeGuess" is a sufficient name - calling "makeGuess" tells the computer to make a guess. Is the if-test in the function necessary? Your while loops and if tests will take care of that in main(), and having that in theGuess is unnecessary and confusing. Next, there's no reason to have theGuess() assign the result of rand()%100+1 to guess. You can simply return that value directly, like so: return rand()%100+1; If you do that, then we see there's no need for guess in that function at all, so we can remove the parameter guess and make the function just int theGuess().

Now how do we use "theGuess" (or "makeGuess" if you followed my advice)? You've currently got "int theGuess(int guess);" in main(). This is almost right. This is called a forward declaration. What it does is let main() know that there will be a function called "theGuess" later on. Without it, we can't use theGuess() since it's below main(), and won't have been "read" by the time main gets "read." However, we need to move it outside of main(). I'd put it just a line or two above main(). Furthermore, we've removed the guess parameter from "theGuess" so we need to make the forward declaration reflect that: int theGuess(); Or if you've renamed it: int makeGuess();.

Now we can actually call theGuess(). What this means is we can tell the computer to run the code inside theGuess() and then we are given back whatever theGuess() returns to us. This means we can write: guess = theGuess(); We call the function theGuess(), and we assign theGuess()'s output (what it returns) to guess. Now we have made a guess, and stored it in the variable guess.

Now, we can see that both input and guess are only ever used inside of main(). This means we can remove them both as global variables outside of main() and put them both inside of main. In general, you want to declare your variables close to where you use them; so declaring inside of main is preferable to declaring outside of main.

Finally, you've got an extra pair of curly brackets { and } around your while(guess != input) loop. This is actually OK in C++, you can have extra brackets so long as they are paired up properly, but it is rarely used, could confuse other readers of your code, and in this case is entirely unnecessary. I would remove those brackets (and make sure to double-check indentation after doing so!).

It think you'll find all your problems are solved with these corrections. To be specific:
Problem 1: See the srand() posts above.
Problem 2: It's because you didn't call theGuess(), you only had a forward declaration of it.
Problem 3: Same as Problem 2, you were never actually changing guess so guess never would equal input.

Good luck, and I hope that helps!

Share this post


Link to post
Share on other sites
thanks a lot everyone!!!
I have no idea what was wrong but my compiler was only indenting half the stuff for some reason!!!

revised code still won't work!!! (infinite loop because of non-random nos.)

#include <iostream>
#include <cstdlib>
#include <ctime>

using namespace std;

int main()
{

srand(time(0));

int number;
int guess=rand()%100+1;

cout<<"\t\tGUESS MY NUMBER\n\n";
cout<<"enter a number between 1 and 100 for the computer to guess: \n";
cin>>number;
while (number>100 || number<0)
{
cout<<"Enter a number between 1 and 100: ";
cin>>number;
}
if (number<=100 && number>=1)
{
while(true)
{
cout<<"Is it: " <<guess <<"\?\n";
if (guess!=number)
continue;
}
}

if (guess==number)
{
cout<<"your number was " <<number;
}
}



Thanks again!!!

Share this post


Link to post
Share on other sites
Let's check how the program executed:

First a guess is made (let's say 20). Then another number in inserted into the program, for the user to choose. Let's say that number is 50. After a range check (yes, 50 is between 0 and 100), the program makes a guess, let's say 20 and starts a loop. The program outputs the current guess ("Is it: 20?"). The program checks if 20 == 50. It isn't, so the loop starts over again, without changing the guess.

Sample output would be:
      GUESS MY NUMBER
Enter a number between 1 and 100: 50
Is it: 20? // No, it isn't.
Is it: 20? // No, it isn't.
Is it: 20? // No, it isn't.
Is it: 20? // No, it isn't.
Is it: 20? // You get the point.
// etc...



You must make a new guess (a new random guess), every time you begin your loop:

while(true)
{
guess = rand()%100+1; // Make a new guess
cout<<"Is it: " <<guess <<"\?\n";

if (guess!=number)
continue;
}


Emiel

Share this post


Link to post
Share on other sites
Indentation looks good now, and that always helps a lot! Some more comments:

The "if (number<=100 && number>=1)" test is not necessary - the loop directly above it guarantees that number is within those boundaries. I'd just remove it.

Your main problems are with the while(true) {...} loop. First while(true) is going to run forever - or at least until one of a couple things happen. Most commonly, you could return out of the function, thus breaking the loop. You could break out of the loop. Or an exception could be thrown (if you don't know what this means, don't worry - it's not really relevant). None of these are happening, so your program will continue indefinitely.

It looks kind of like you've tried to stop the loop with your continue, but that's not what continue does; it stops the current iteration of the loop, skipping onto the next one. Continue never stops the entire loop, just whatever is left of the current time through it. Instead, you want to use break, which would stop the loop and continue onwards below it. However, then your if condition is broken; we need to stop when guess == number, not when guess != number.

However, there's a better way than break. As with your while loop for asking for a number, you can give while a test condition, which it will only repeat so long as that condition is true. In this case, we want to keep guessing so long as the guess is wrong. That means we need: while(guess != number) {...}. Then, when we get a guess that does equal number, the loop stops on it's own, and until then we keep on guessing.

Other than that, Seol's advice should be all you need to make this work fine. Currently, in the while(true) loop, you don't ever change guess. This means, if it's wrong at the beginning, it will always be wrong and the computer will never figure it out. In the beginning, you have int guess=rand()%100+1; which is a good way to get a random number. What you need to do is make a new guess every time through your guessing loop. Try assigning to guess again every time through the loop.

You're almost there, so I hope that helps!

Share this post


Link to post
Share on other sites
Thanks everyone!!! finally got it: code below:


#include <iostream>
#include <cstdlib>
#include <ctime>

using namespace std;

int main()
{

srand(time(0));

int number;
int guess=rand()%100+1;

cout<<"\t\tGUESS MY NUMBER\n\n";
cout<<"enter a number between 1 and 100 for the computer to guess: \n";
cin>>number;
while (number>100 || number<0)
{
cout<<"Enter a number between 1 and 100: ";
cin>>number;
}
while(guess!=number)
{
guess=rand()%100+1;
cout<<"Is it: " <<guess <<"\?\n";
}

if (guess==number)
{
cout<<"your number was " <<number;
}
}

Share this post


Link to post
Share on other sites
Looks very good. Now, some cleaning up. Comments are points of interest.

#include <iostream>
#include <cstdlib>
#include <ctime>

using namespace std;

int main()
{// Why a blank line?

srand(time(0));

int number;
int guess=rand()%100+1; // When you start guessing, the guess is set automatically. No need to set it here. See (A).

cout<<"\t\tGUESS MY NUMBER\n\n";
cout<<"enter a number between 1 and 100 for the computer to guess: \n";
cin>>number;
while (number>100 || number<0)
{
cout<<"Enter a number between 1 and 100: ";
cin>>number;
}
while(guess!=number) // Why another indentation?
{
guess=rand()%100+1; // (A)
cout<<"Is it: " <<guess <<"\?\n";
}

if (guess==number) // When you end your while loop, guess is always equal to number. No need to check it another time.
{
cout<<"your number was " <<number;
}
}


Emiel

Share this post


Link to post
Share on other sites
Quote:
Original post by soitsthateasy
Thanks everyone!!! finally got it: code below:

*** Source Snippet Removed ***


you can set the guess 0, you shoulde check for guess < 1 instead of 0. and it will enter an infinite loop ^^

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this