Let's simplify the example. Your code does not work for the same reason that this one will not:
int x = 2;
if(x == 1 || 2) { ... }
That's because || operator does not concatenate in such way. The above example evaluates the x == 1 (false) and that ORs it (||) with 2, like so:
if(false || 2) { ... } // after evaluating of x == 1
We now have false || 2. In C++, any non-zero numeric value evaluates to true, so we end up with false || true, which is true.
But it seems like it works in this example, right?
It only appears to. Replace the 2 with 3 and the code will still be evaluated to true, even though x is neither 1 nor 3.
How to fix the issue?
In case of small combinations, one should just correctly provide all the expressions:
if(x == 1 || x == 2) { ... }
instead of:
if(x == 1 || 2) { ... }
So in your example you would need:
if(classSelection != "C" || classSelection != "c" || classSelection != "F" || ... )
But that still wouldn't do what you want. You don't want to know whether classSelection is not equal to, for instance, "C" or not equal to "c". You want to know if it's not equal to "C" and not equal to "c", so you'd actually want:
if(classSelection != "C" && classSelection != "c" && classSelection != "F" && ... )
And your example would require some typing. Another approach, slightly less eficient, but arguably more readable, would be to store all the possible matches in a data structure and use a standard algorithm, such as std::all_of:
#include <iostream>
#include <algorithm>
#include <vector>
#include <string>
int main() {
std::vector<const char*> not_accepted= {
"C", "c", "F", "f", "P", "p", "R", "r", "W", "w"
};
std::string choice = "f"; // provided by user
bool valid = std::all_of(not_accepted.cbegin(), not_accepted.cend(),
[&](auto arg){
return choice != arg;
});
std::cout << "valid? - " << std::boolalpha << valid;
}
std::all_of takes a range and a predicate and returns true, if all the elements satisfy the predicate. Here, the range is our not_accepted vector and our predicate is represented as a lambda, which will compare every single element of that vector with our choice string.
To optimise, one could replace std::vector with a std::set or std::unordered_set and check for elements being present in a set. That would get rid of an algorithm call:
#include <iostream>
#include <unordered_set>
#include <string>
int main() {
std::unordered_set<const char*> not_accepted = {
"C", "c", "F", "f", "P", "p", "R", "r", "W", "w"
};
std::string choice = "f";
bool valid = not_accepted.find("f") == not_accepted.end();
std::cout << "valid? - " << std::boolalpha << valid;
}
