tags:

views:

193

answers:

7

I have a novice question since I just started learning C++ not too long ago. I have the following if statements two of which don't seem to work. I don't get why it works when I try to compare it to a single character "y" or "n" but not when I try to compare it to two characters in one else if statement. The last question I have is if there's a better cleaner way to write this or if this acceptable for a simple prompt check?

getline(cin,somestr);

if(somestr.empty()){
//do this
}
else if (somestr == "y" || "Y"){
//do something else
}
else if (somestr == "n" || "N"){
//do something else
}
else{}
+10  A: 

You would do it like this:

else if(somestr == "y" || somestr == "Y")
jMerliN
To elaborate: The `||` operator takes an expression on each side. The expression "N" alone in a boolean context is equivalent to `"N" != 0`. So the original code was equivalent to `if (somestr == "n" || "N" != 0)`.
Chuck
What a way to make myself feel dumb in front of the world! Of course this was it! I can't believe I made such a silly mistake. I'm may be new to C++ but not that much! :P
Tek
Well, it's not *that* silly, I've seen many beginners make that mistake in many languages. Because when you speak, you tend to say it as "if x equals 1 or 2", but it's rare to hear someone say "if x equals 1 or equals 2", so people try to write the code the way they hear it.
FrustratedWithFormsDesigner
@Chuck Thanks chuck, for the more technical answer. Although I get it without going into that much depth, I appreciate your explanation a lot :) The more you know!
Tek
@Frustrated Yes, for some reason I could not get that sentence out of my head. I should say things out loud more often :laugh:
Tek
@Chuck Technically, `"N"` alone in a boolean context (the `||` as you point out forces it into that situation) acts as a character, an integral type, and in C++, an integral type evaluates to `false` if zero and `true` otherwise. Therefore that statement is equivalent to `if( somestr == "n" || true )`, which will always be equivalent to `if(true)`, so no `else` case following would execute.
jMerliN
@jMerliN: That's not exactly right. "N" does not act as a character, it acts as a pointer. Try it out: `if (false || '\0') cout << "this won't print"; if (false || "\0") cout << "string not the same as char";`
Chuck
@Chuck This is true. So what I said is true, and for the same reason I gave, but only because these pointers must by definition evaluate to an integral that is non-zero. It's nice to get it right accidentally sometimes.
jMerliN
A: 

You should write two comparisons

somestr == "n" ||  somestr=="N"
Tom
A: 

First of all, you need to fix this:

(somestr == "y" || "Y")  to (somestr == "y" || somestr == "Y")

and

(somestr == "n" || "N")  to (somestr == "n" || somestr == "N")

Otherwise, these expressions always evaluate to true because any string by itself such as "y" evaluates to true along with anything else other than 0.

Secondly, you may want to ask for input again if it is not "y", "Y", "n" or "N". You could use a do-while loop to accomplish this.

mouche
+7  A: 
if (somestr == "y" || "Y"){

Keep in mind, in C++ 0 is false and everything else is true. Since "Y" is not zero, it's true. So what you've really written is: if (something || true). Which is always true.

jeffamaphone
+2  A: 

I would do something like

else if(someFunctionThatConvertsToUpper(somestr) == "Y")
Fernando
Me too, but I'd go lower instead of upper. Personal preference. :)
jeffamaphone
+7  A: 

Unfortunately, the language doesn't give you an easy way to check a variable against a set of possibilities. You have to do each test individually or use a switch statement. So, either of the following code samples would be a valid solution for your problem:

else if (somestr == 'y' || somestr == 'Y'){
//do something else
}
else if (somestr == 'n' || somestr == 'N'){
//do something else
}


switch (somestr) {
    case 'y':
    case 'Y':
        // do something
        break;

    case 'n':
    case 'N':
        // do something
        break;

    default:
        break;
}

Alternatively, you can clean up your code a bit by reducing some of your logic (assuming somestr is a char):

// Convert to uppercase first and only one comparison is needed
else if (toupper(somestr) == 'Y'){
//do something else
}
else if (toupper(somestr) == 'N'){
//do something else
}
bta
Sweet thanks for this, really helpful!
Tek
We generally want to avoid fall-through cases in switches, especially if we don't know how to properly use them as they're the source of some very obscure errors. So often is 'break' used in every case that when one is omitted it's generally unclear if this is intentional without the customary `/* FALLTHROUGH */` explicitly letting any other reader of that code know the behavior is intended.
jMerliN
Picked this as the answer for effort and being the most helpful answer :)
Tek
@JMerliN Very interesting to know tidbits like those, good comment!
Tek
@jMerliN: I put 'break' in front of every 'case' statement on the same line. This means that any 'case' that doesn't have a preceding 'break' stands out.Perhaps I should force that with this!: #define case break; case:
Aaron McDaid
You cannot use the switch with strings --as the question states-- but only with plain chars.
David Rodríguez - dribeas
@Tek: somestr is a string. To test if the first character matches any in a set, you could do this: char somechar = somestr.at(0); if(strchr("yY", somechar)) { /* do stuff */ }This works because strchr returns NULL if it can't find somechar in the string. This is overkill, and would only make sense if your set of 'yes' options was quite long.
Aaron McDaid
+1  A: 

Anther option, especially if you're only expecting characters - it looks like y or n for yes or no - is to read in a char, not a string, and use a switch statement.

char somechar;
cin.get(somechar);

switch(somechar){
  case 'y' : case 'Y':
    //do something
  break;
  case 'n' : case 'N':
    // do something else
    // break;
  default:
    // do something else
}
roviuser
@bta types a liiiitle bit faster than me.
roviuser