if/ else if will not return else if when it is true

I have a code which creates a random number, the user has to guess it in 10 tries. The first cout is the option to "Cheat? (Y/N)" which will display the secret number (the random number), mostly just for me to see whats happening.

If the user selects Y the number is displayed. But also it will be displayed if they select N. This is one point of confusion.

After this they are prompted to enter a guess. If the guess is wrong they will be asked if they want to try again. Whether they say Y or N, the program just quits and outputs nothing.

Here is the code:

Code:

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

int main() 
{ 
	srand ( time(NULL) );
	int number = (rand() % 15) + 1;
	int guess; 
	int trycount = 0;
	char choice;
	cout << "Cheat? (Y/N) \n";
	cin >> choice;
	if (choice == 'Y' || 'y')
		cout << "The secret number is " << number << ".\n";
	else if (choice == 'N')
		cout << "Good for you.";

	cout << "Please enter a guess: ";
	cin >> guess;
	while (guess != number && trycount < 10) 
		{
			int choice;
			cout << "Wrong, guess again? (Y/N): ";
			cin >> choice;
			if (choice == 'Y')
			{
				cout << "Guess again: ";
				cin >> guess;
			}
			else if (choice != 'Y')
				break;
			{
			if (guess < number) 
				cout << "Too low \n";
			else if (guess > number)
				cout << "Too high \n";
			else (guess == number);
				cout << "You guessed the number!";
			}
			trycount++; 
		} 
return 0;
}

Here are some outputs/inputs:

Cheat? (Y/N)
Y
The secret number is 13.
Please enter a guess: 6
Wrong, guess again? (Y/N): Y

The Debugger has exited with status 0.

----------

Cheat? (Y/N)
N
The secret number is 11.
Please enter a guess: 7
Wrong, guess again? (Y/N): N

The Debugger has exited with status 0.

Can anyone give me a hint about what I am missing? I feel like it is a simple solution but I've tried it for a bit and I can't seem to figure out what is going wrong. Thank you!

Code:

if (choice == 'Y' || 'y')

Your attempt at using your knowledge of English to say something in C++ is natural and understandable. But in C++, this line is read as "if choice=='Y', or if 'y' is nonzero", which it is, and so the "else if" never happens.

Code:

else if (choice != 'Y')
				break;
			{

Why do you open scope after breaking? Did you mean to enclose the "break" only in the brackets?

Ah, ok, so in order to accept both uppercase and lowercase y, I need to code:

Code:

if (choice == 'Y' || choice == 'y')

correct? I forgot about the syntax involved with the Or operator.

Why do you open scope after breaking? Did you mean to enclose the "break" only in the brackets?

Not sure what I was thinking there. break will end the if/else if statement right? So if I remove the bracket the program should just continue to the next if/else if/else statement?

So now the only issue is the while loop.

Code:

cout << "Please enter a guess: ";
	cin >> guess;
	while (guess != number && trycount < 10) 
		{
			int choice;
			cout << "Wrong, guess again? (Y/N): ";
			cin >> choice;
			if (choice == 'Y')
			{
				cout << "Guess again: ";
				cin >> guess;
			}
			else if (choice != 'Y') // I think that syntax is off in these next couple lines but I am not sure exactly what is incorrect
				break;
			if (guess < number) 
				cout << "Too low \n";
			else if (guess > number)
				cout << "Too high \n";
			else (guess == number);
				cout << "You guessed the number!";
			}
			trycount++; 
	return 0;
		}

Looks a lot better. For the sake of logic flow, you might consider putting all of the stuff happening in your loop that is after the "break" -- you might consider putting all of that in the "if (choice == 'Y')" block. This way it is clear that those things happen only when the user says 'yes'. Of course that is the case now, but it is clearer the way I said.

Code:

#include <stdlib.h>
#include <iostream>
#include <cstdlib>
#include <ctime>

int main() 
{
	using namespace std; // Just my preference. Others disagree.

	srand ( time(NULL) );
	int number = (rand() % 15) + 1;
	int guess; 
	int trycount = 0;
	char choice;
	cout << "Cheat? (Y/N) \n";
	cin >> choice;
	if (choice == 'Y' || choice == 'y')
		cout << "The secret number is " << number << ".\n";
	else
		cout << "Good for you.";

	cout << "Please enter a guess: ";
	cin >> guess;
	while (guess != number && trycount < 10) 
	{
		int choice;
		cout << "Wrong, guess again? (Y/N): ";
		cin >> choice;
		if (choice == 'Y')
		{
			cout << "Guess again: ";
			cin >> guess;
			if (guess < number) 
				cout << "Too low \n";
			else if (guess > number)
				cout << "Too high \n";
			else
				cout << "You guessed the number!";
			trycount++; 
		}
		else
			break;
	} 
	return 0;
}

Originally Posted by nnizzle

Code:

else (guess == number);
	cout << "You guessed the number!";

That isn't doing what you think it is.

Also unnecessary: redeclaring 'choice' inside your loop. And this doesn't require an else if:

Code:

else if (choice == 'N')
	cout << "Good for you.";

(You're not handling lowercase 'n' anyway.)

Quote:
Originally Posted by nnizzle
Code:

else (guess == number);
cout << "You guessed the number!";

That isn't doing what you think it is.

What is it doing? I know for if/else the inside of the parentheses has to be a Boolean true statement for it to run, or else it will be skipped over. Are you saying that since all the other options (higher than the number and lower than the number) have been dealt with I can just write else?

Code:

if (guess < number) 
				cout << "Too low \n";
			else if (guess > number)
				cout << "Too high \n";
			else (guess == number);
				cout << "You guessed the number!";
			trycount++;

So this is a nested if function, correct? Why does the final else have to be outside the function body, wouldn't it accomplish the same thing either way?

Thank you both for all your help so far.

What is it doing?

All kinds of stuff I missed.

There's the semicolon, too.

Code:

else (guess == number);
	cout << "You guessed the number!";
//This code is equivalent to:
//else {}
//cout << "You guessed the number!";

should be

Code:

else if(guess == number)
	cout << "You guessed the number!";

Or, since guess==number is the only alternative after the previous if statements,

Code:

else
	cout << "You guessed the number!";

Thank you codemonkey. I still am a semicolon noob. I finally figured out why it kept quitting, a stupidly simple answer:

Code:

int choice;
		cout << "Wrong, guess again? (Y/N): ";

But the input is a character. So I changed to char and it seems to be gold.

Here is the final code, I added some things but it seems to be working perfect now. Thanks for all your help!

Code:

#include <stdlib.h>
#include <iostream>
#include <cstdlib>
#include <ctime>
using namespace std;

int main() 
{ 
	srand ( time(NULL) );
	int number = (rand() % 15) + 1;
	int guess; 
	int trycount = 0;
	char choice;
	cout << "Cheat? (Y/N) \n";
	cin >> choice;
		if (choice == 'Y' || choice == 'y')
			cout << "The secret number is " << number << ".\n";
		else if (choice == 'N' || choice == 'n')
			cout << "Good for you.\n";
	
	cout << "Please enter a guess: ";
	cin >> guess;
		if (guess < number)
			cout << "Too low.\n";
		else if (guess > number)
			cout << "Too high.\n";
		else
			cout << "Right!\n";
	
	while (guess != number && trycount < 5) 
	{
		char choice;
		cout << "Guess again? (Y/N): ";
		cin >> choice;
			if (choice == 'Y')
			{
				cout << "Guess again: ";
				cin >> guess;
					if (guess < number) 
						cout << "Too low \n";
					else if (guess > number)
						cout << "Too high \n";
					else
						cout << "You guessed the number!\n";
				trycount++;
			}
	}
	if (trycount == 5)
		cout << "Too many tries!  Better luck next time. ";
	else
		cout << "Bye!";
	return 0;
}

Please, if you have any recommendations (even though it works) feel free to offer them.

Also codemonkey, what is the advantage/disadvantage of having "using namespace std" inside or outside the int main function body?

Also codemonkey, what is the advantage/disadvantage of having "using namespace std" inside or outside the int main function body?

using namespace std; is a nice shorthand, but generally it's a good habit to fully qualify library stuff, so that there isn't any name-clashing with your code, or with code in some library you're using. For example, common words like pair and set exist in namespace std. So it prevents headaches to be sure you mean std::pair.

I put the using namespace std; inside of a function sometimes if I know that it's very unlikely that I'll have name clashes in that code, and if it makes my life that much easier (std:: is only 5 chars). If you put it in the global scope, then it applies everywhere (throughout the compilation unit) and God knows what can happen.

Don't check your char type variables with a simple '==' equality sign.

Use strcmp to check your variables, here is your problem:

Code:

if ( choice == "Y" )
{
   ...
}
else ( choice == "N" )
{
  ...
}

Here is that code, but fixed:

Code:

if ( !strcmp(choice, "Y") )
{
  ...
}
else ( !strcmp(choice, "N") )
{
  ...
}

strcmp is a function designed to compare characters together, if the characters match strcmp returns 0.

More information here: strcmp, wcscmp, _mbscmp (CRT)

Originally Posted by iPromise

Don't check your char type variables with a simple '==' equality sign.

Actually, it is fine to use operator== to compare chars. What you have in mind are null terminated strings, but those are not what nnizzle compared.

However, do note that it is a C++ idiom to compare any type with the equal operator. std::string overloads operator == to provide equality check with it, for example.
Null terminated strings is not a C++ type, but a C type, and hence this idiom doesn't apply to it. Nevertheless, they should be avoided if possible.