0

My if statements are getting hit no matter the input. I can't wrap my head around why. Here is the code:

void Novice::selection()
{
    char selection, shift;
    cout << "Please select a section to run: A - Home Row, B - Bottom Row, C - Top Row, D - Pointer Fingers, E - Right Pinky;" << endl;
    cin >> selection; 
    selection = toupper(selection);
    if (selection != 'A' || 'B' || 'C' || 'D' || 'E') {
        cout << "Invalid Input.  Please select again" << endl;
        cin >> selection; 
    }
    if (selection == 'A' || 'B' || 'C') {
        cout << "you're here" << endl;
    }

If input is 'A', the first if statement triggers, if I then put in A again second if statement triggers as well. Any help would be appreciated.

phuclv
  • 37,963
  • 15
  • 156
  • 475
Matthew
  • 49
  • 7
  • 1
    Tell me what the `||` operator does. – eesiraed May 10 '20 at 18:00
  • The expression `'B'` evaluates to the character code 66 (assuming ASCII is being used), which is nonzero, so it is true. The result of a `||` operation is true if either operands are true. That means your if condition will always be true. – Andreas Wenzel May 10 '20 at 18:02
  • duplicates: [If always returns true](https://stackoverflow.com/q/32035762/995714), https://stackoverflow.com/q/59568905/995714, [Why is if (typeCar != 'X' || 'S' ||'L') is evaluating to always true](https://stackoverflow.com/q/47152966/995714) – phuclv Oct 06 '21 at 04:38
  • Does this answer your question? [Character check in file always returns true](https://stackoverflow.com/questions/56605850/character-check-in-file-always-returns-true) – phuclv Oct 06 '21 at 04:39

1 Answers1

2

This is not how logical operators work in c++. To compare against multiple values, you need to do:

 if (selection == 'A' || selection == 'B' || selection == 'C') {
   // ...
}

Note that your first if is incorrect, even if you use the fix above. If you check whether a value is not equal to several other values, this will always be true. That condition probably needs to be something like:

if (selection != 'A' && selection != 'B' && 
    selection != 'C' && selection !=  'D' && selection != 'E') {
  // ...
}

Alternatively, for the first if condition, you can use a switch statement, like this:

switch ( selection )
{
case 'A':
case 'B':
case 'C':
case 'D':
case 'E':
    break;
default:
    cout << "Invalid Input.  Please select again" << endl;
    cin >> selection;
}
cigien
  • 57,834
  • 11
  • 73
  • 112
  • Or simply use a `switch` statement. – Andreas Wenzel May 10 '20 at 18:00
  • @AndreasWenzel Not sure a `switch` is actually better for the OP's example. – cigien May 10 '20 at 18:01
  • 1
    Not very useful for correcting this one, though: `if (selection != 'A' || 'B' || 'C' || 'D' || 'E')` (need to use `&&`). – Adrian Mole May 10 '20 at 18:02
  • 1
    True, added an explanation for that. – cigien May 10 '20 at 18:05
  • Ahh yes... they are all now working. thank you very much figured it was something simple that I missed.. thank you thank you.. – Matthew May 10 '20 at 18:06
  • 1
    No worries, happy to help :) Consider [accepting](https://stackoverflow.com/help/someone-answers) the answer when you can. Also, take the [tour](https://stackoverflow.com/tour), you'll get a badge for that :) – cigien May 10 '20 at 18:07
  • @cigien: A switch statement seems perfect for the situation of the OP. Do you mind if I edit your answer to add my switch statement? Feel free to revert my edit if you don't like it. – Andreas Wenzel May 10 '20 at 18:10
  • @AndreasWenzel Of course, you don't need my permission. Go right ahead :) If you feel it's a sufficiently *different* answer, you can add an answer yourself. – cigien May 10 '20 at 18:11
  • @cigien: I have completed my edit. I guess it's a matter of taste whether to use `if` or `switch`. – Andreas Wenzel May 10 '20 at 18:14
  • @AndreasWenzel Yes, that's a reasonable solution for the first `if`, thanks. I edited it slightly to make that clearer. – cigien May 10 '20 at 18:18