How to improve logic to check whether 4 boolean values match some cases
I have four bool
values:
bool bValue1;
bool bValue2;
bool bValue3;
bool bValue4;
The acceptable values are:
Scenario 1 | Scenario 2 | Scenario 3
bValue1: true | true | true
bValue2: true | true | false
bValue3: true | true | false
bValue4: true | false | false
So, for example, this scenario is not acceptable:
bValue1: false
bValue2: true
bValue3: true
bValue4: true
At the moment I have come up with this if
statement to detect bad scenarios:
if(((bValue4 && (!bValue3 || !bValue2 || !bValue1)) ||
((bValue3 && (!bValue2 || !bValue1)) ||
(bValue2 && !bValue1) ||
(!bValue1 && !bValue2 && !bValue3 && !bValue4))
{
// There is some error
}
Can that statement logic be improved/simplified?
I would aim for readability: you have just 3 scenario, deal with them with 3 separate ifs:
bool valid = false;
if (bValue1 && bValue2 && bValue3 && bValue4)
valid = true; //scenario 1
else if (bValue1 && bValue2 && bValue3 && !bValue4)
valid = true; //scenario 2
else if (bValue1 && !bValue2 && !bValue3 && !bValue4)
valid = true; //scenario 3
Easy to read and debug, IMHO. Also, you can assign a variable whichScenario
while proceeding with the if
.
With just 3 scenarios, I would not go with something such "if the first 3 values are true I can avoid check the forth value": it's going to make your code harder to read and maintain.
Not an elegant solution maybe surely, but in this case is ok: easy and readable.
If your logic gets more complicated, throw away that code and consider using something more to store different available scenarios (as Zladeck is suggesting).
I really love the first suggestion given in this answer: easy to read, not error prone, maintainable
(Almost) off topic:
I don't write lot of answers here at StackOverflow. It's really funny that the above accepted answer is by far the most appreciated answer in my history (never had more than 5-10 upvotes before I think) while actually is not what I usually think is the "right" way to do it.
But simplicity is often "the right way to do it", many people seems to think this and I should think it more than I do :)
I would aim for simplicity and readability.
bool scenario1 = bValue1 && bValue2 && bValue3 && bValue4;
bool scenario2 = bValue1 && bValue2 && bValue3 && !bValue4;
bool scenario3 = bValue1 && !bValue2 && !bValue3 && !bValue4;
if (scenario1 || scenario2 || scenario3) {
// Do whatever.
}
Make sure to replace the names of the scenarios as well as the names of the flags with something descriptive. If it makes sense for your specific problem, you could consider this alternative:
bool scenario1or2 = bValue1 && bValue2 && bValue3;
bool scenario3 = bValue1 && !bValue2 && !bValue3 && !bValue4;
if (scenario1or2 || scenario3) {
// Do whatever.
}
What's important here is not predicate logic. It's describing your domain and clearly expressing your intent. The key here is to give all inputs and intermediary variables good names. If you can't find good variable names, it may be a sign that you are describing the problem in the wrong way.