Can someone tell me what's wrong with my if statement?

[Scythe37]

I am writing a rock paper scissors program for my homework and I can't figure out why it won't compile. I am getting an error message about my if statement, but I don't understand what's wrong. Can someone help me please?


#include <iostream>
#include <string>
using namespace std;
int main()
{
string move1;
string move2;
char R;
char r;
char P;
char p;
char S;
char s;

cout << "Player One, please enter your move: " << move1 << endl << "('P' for Paper, 'R' for Rock, 'S' Scissor)" << endl;
cin >> move1;
cin.ignore(1000,10);
cout << "Player Two, please enter your move: " << move2 << endl << "('P' for Paper, 'R' for Rock, 'S' Scissor)" << endl;
cin >> move2;
cin.ignore(1000,10);


if (move1 != 'R' || 'r' || 's' || 'S' || 'p' || 'P')
{ cout << "Player 1 has entered and invalid move.";
}
return 0;
}

[kypeswith]

i think what you need to use is a switch...But, if you want, you can use something like if(move1 !='R' || move1 != 'r' || move1 != 's' ... etc.. although i'm not sure how that will work out for you ;) try reading about the switch statement

[Return 0]

To start with (does not include all the errors):
1. What do you need all those chars for?
2. In both of your cout statements you are attempting to output move1 and move2 which have been uninitialized and have no value.
3. You are using a string to capture a single character entry rather than using a char and in your if statement you are comparing that string to a char, which won't work.

You could use a switch statement, something like this were default catches the exceptions:

switch( move1 )
{
case 'R':
	// do something
	break;
case 'r':
	// do something
	break;
default:
	cout << "Player 1 has entered an invalid move.";
}

[Scythe37]

Thank you kypeswith, I forgot that I needed to repeat "move1 !=" between operators.

Thank you Return 0, I didn't know that I could declare char like that.

[Disch]

This is a common mistake with ||. People think "I don't want move to be 'R' or 'r' or 's' or..." so that's how they construct their if statements in the code.

This is not how || works. || evaluates the expressions on either side of it and returns true if either one of them is true. It does not "combine" operands for one expression.

Example:

if(m != 'R' || 'r')

The code will see if m != 'R' is true

then it will see if 'r' is true (not m != 'r'!)

Since any nonzero value is treated as 'true', and since 'r' is a nonzero integer/character, this expression will always be true, and thus the if() statement will always execute.

What you wanted to do is probably something like:

if(move1 != 'R' && move1 != 'r' && move1 != 's' ...

But of course this is tedious and ugly. kypeswith's suggestion of using a switch() would be my suggestion as well.

switch(move1)
{
case 'R': case 'r': case 's': case 'S': case 'p': case 'P':
  DoSomething();
  break;

default:
  InvalidMove();
  break;
}

edit -- but of course you can't use switch with a string -- nor can you use == to compare a string and char (at least I don't think so). So yeah you have other problems here besides just this.

kypeswith's other suggestion is erroneous though:

if(move1 != 'R' || move1 != 'r')

will always come back true because the only way it can be false is for move1 to equal both 'R' and 'r' (which is of course, impossible)

[kypeswith]

the logical AND (&&) would be what you need

also, i like to use char* myvar as a string, really i think it's a pointer to a dynamically allocated char array though.. it will work just fine to hold a string of any size, i've never used a string variable before, but it's probably very similiar, in that you use it like a normal variable:

char * myvar = "insert any size string you want to.";
char * other = "there is no kypeswith";
myvar = other; // remember that the variable on the left hand side of the = is the one that gets modified, although i think it just changes the address of the pointer myvar to the address of other, but i could be wrong.

cout << myvar;
cout << other;

EDIT: don't worry about it acting like a pointer.. ( when you use the assignment statement it doesn't change the memory address the pointer points to, it's just like a variable.. quite handy)