How to improve logic to check whether 4 boolean values match some cases

Question

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.

We can use a Karnaugh map and reduce your scenarios to a logical equation. I have used the Online Karnaugh map solver with circuit for 4 variables.

This yields:

Changing A, B, C, D to bValue1, bValue2, bValue3, bValue4, this is nothing but:

bValue1 && bValue2 && bValue3 || bValue1 && !bValue2 && !bValue3 && !bValue4

So your if statement becomes:

if(!(bValue1 && bValue2 && bValue3 || bValue1 && !bValue2 && !bValue3 && !bValue4))
{
    // There is some error
}

• Karnaugh Maps are particularly useful when you have many variables and many conditions which should evaluate true.
• After reducing the true scenarios to a logical equation, adding relevant comments indicating the true scenarios is good practice.

The real question here is: what happens when another developer (or even author) must change this code few months later.

I would suggest modelling this as bit flags:

const int SCENARIO_1 = 0x0F; // 0b1111 if using c++14
const int SCENARIO_2 = 0x0E; // 0b1110
const int SCENARIO_3 = 0x08; // 0b1000

bool bValue1 = true;
bool bValue2 = false;
bool bValue3 = false;
bool bValue4 = false;

// boolean -> int conversion is covered by standard and produces 0/1
int scenario = bValue1 << 3 | bValue2 << 2 | bValue3 << 1 | bValue4;
bool match = scenario == SCENARIO_1 || scenario == SCENARIO_2 || scenario == SCENARIO_3;
std::cout << (match ? "ok" : "error");

If there are many more scenarios or more flags, a table approach is more readable and extensible than using flags. Supporting a new scenario requires just another row in the table.

int scenarios[3][4] = {
    {true, true, true, true},
    {true, true, true, false},
    {true, false, false, false},
};

int main()
{
  bool bValue1 = true;
  bool bValue2 = false;
  bool bValue3 = true;
  bool bValue4 = true;
  bool match = false;

  // depending on compiler, prefer std::size()/_countof instead of magic value of 4
  for (int i = 0; i < 4 && !match; ++i) {
    auto current = scenarios[i];
    match = bValue1 == current[0] && 
            bValue2 == current[1] && 
            bValue3 == current[2] && 
            bValue4 == current[3];
  }

  std::cout << (match ? "ok" : "error");
}

My previous answer is already the accepted answer, I add something here that I think is both readable, easy and in this case open to future modifications:

Starting with @ZdeslavVojkovic answer (which I find quite good), I came up with this:

#include <iostream>
#include <set>

//using namespace std;

int GetScenarioInt(bool bValue1, bool bValue2, bool bValue3, bool bValue4)
{
    return bValue1 << 3 | bValue2 << 2 | bValue3 << 1 | bValue4;
}
bool IsValidScenario(bool bValue1, bool bValue2, bool bValue3, bool bValue4)
{
    std::set<int> validScenarios;
    validScenarios.insert(GetScenarioInt(true, true, true, true));
    validScenarios.insert(GetScenarioInt(true, true, true, false));
    validScenarios.insert(GetScenarioInt(true, false, false, false));

    int currentScenario = GetScenarioInt(bValue1, bValue2, bValue3, bValue4);

    return validScenarios.find(currentScenario) != validScenarios.end();
}

int main()
{
    std::cout << IsValidScenario(true, true, true, false) << "\n"; // expected = true;
    std::cout << IsValidScenario(true, true, false, false) << "\n"; // expected = false;

    return 0;
}

See it at work here

Well, that's the "elegant and maintainable" (IMHO) solution I usually aim to, but really, for the OP case, my previous "bunch of ifs" answer fits better the OP requirements, even if it's not elegant nor maintainable.

I would also like to submit an other approach.

My idea is to convert the bools into an integer and then compare using variadic templates:

unsigned bitmap_from_bools(bool b) {
    return b;
}
template<typename... args>
unsigned bitmap_from_bools(bool b, args... pack) {
    return (bitmap_from_bools(b) << sizeof...(pack)) | bitmap_from_bools(pack...);
}

int main() {
    bool bValue1;
    bool bValue2;
    bool bValue3;
    bool bValue4;

    unsigned summary = bitmap_from_bools(bValue1, bValue2, bValue3, bValue4);

    if (summary != 0b1111u && summary != 0b1110u && summary != 0b1000u) {
        //bad scenario
    }
}

Notice how this system can support up to 32 bools as input. replacing the unsigned with unsigned long long (or uint64_t) increases support to 64 cases. If you dont like the if (summary != 0b1111u && summary != 0b1110u && summary != 0b1000u), you could also use yet another variadic template method:

bool equals_any(unsigned target, unsigned compare) {
    return target == compare;
}
template<typename... args>
bool equals_any(unsigned target, unsigned compare, args... compare_pack) {
    return equals_any(target, compare) ? true : equals_any(target, compare_pack...);
}

int main() {
    bool bValue1;
    bool bValue2;
    bool bValue3;
    bool bValue4;

    unsigned summary = bitmap_from_bools(bValue1, bValue2, bValue3, bValue4);

    if (!equals_any(summary, 0b1111u, 0b1110u, 0b1000u)) {
        //bad scenario
    }
}

Here's a simplified version:

if (bValue1 && (bValue2 == bValue3) && (bValue2 || !bValue4)) {
    // acceptable
} else {
    // not acceptable
}

Note, of course, this solution is more obfuscated than the original one, its meaning may be harder to understand.

---

Update: MSalters in the comments found an even simpler expression:

if (bValue1&&(bValue2==bValue3)&&(bValue2>=bValue4)) ...