2

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{}
Improve this question
1
  • Funny, I thought that "or" worked like that when I was using BASIC in my childhood. It took me years and an internet access to understand my mistake... Commented Jul 14, 2010 at 20:42

7 Answers 7

Reset to default
9

You would do it like this:

else if(somestr == "y" || somestr == "Y")
Improve this answer
Sign up to request clarification or add additional context in comments.

8 Comments

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).
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
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.
@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!
@Frustrated Yes, for some reason I could not get that sentence out of my head. I should say things out loud more often :laugh:
|
7 
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.

Improve this answer

Comments

6

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
}
Improve this answer

6 Comments

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.
Picked this as the answer for effort and being the most helpful answer :)
@JMerliN Very interesting to know tidbits like those, good comment!
@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:
You cannot use the switch with strings --as the question states-- but only with plain chars.
|
1

I would do something like 

else if(someFunctionThatConvertsToUpper(somestr) == "Y")
Improve this answer

1 Comment

Me too, but I'd go lower instead of upper. Personal preference. :)
1

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
}
Improve this answer

1 Comment

@bta types a liiiitle bit faster than me.
0

You should write two comparisons 

somestr == "n" ||  somestr=="N"
Improve this answer

Comments

0

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.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.