How to improve logic to check whether 4 boolean values match some cases
up vote
48
down vote
favorite
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?
c++ if-statement
|
show 17 more comments
up vote
48
down vote
favorite
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?
c++ if-statement
5
I would use a table instead of complexif
statement. Additionally, as these are boolean flags, you can model each scenario as a constant and check against it.
– Zdeslav Vojkovic
yesterday
3
if (!((bValue1 && bValue2 && bValue3) || (bValue1 && !bValue2 && !bValue3 && !bValue4)))
– mch
yesterday
8
what are the scenarios actually? Often things get much simpler if you just give stuff proper names, egbool scenario1 = bValue1 && bValue2 && bValue3 && bValue4;
– user463035818
yesterday
2
Using meaningful names, you can extract each complex condition into a method and call that method in if condition. It would be much more readable and maintainable. e.g. Take a look at the example provided in the link.refactoring.guru/decompose-conditional
– Hardik Modha
yesterday
2
@HardikModha Yeah, I see how I might be able to implement that in the code I have now for modular clarity. Thank you.
– Andrew Truckle
yesterday
|
show 17 more comments
up vote
48
down vote
favorite
up vote
48
down vote
favorite
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?
c++ if-statement
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?
c++ if-statement
c++ if-statement
edited yesterday
Dukeling
44.5k1060105
44.5k1060105
asked yesterday
Andrew Truckle
5,04732145
5,04732145
5
I would use a table instead of complexif
statement. Additionally, as these are boolean flags, you can model each scenario as a constant and check against it.
– Zdeslav Vojkovic
yesterday
3
if (!((bValue1 && bValue2 && bValue3) || (bValue1 && !bValue2 && !bValue3 && !bValue4)))
– mch
yesterday
8
what are the scenarios actually? Often things get much simpler if you just give stuff proper names, egbool scenario1 = bValue1 && bValue2 && bValue3 && bValue4;
– user463035818
yesterday
2
Using meaningful names, you can extract each complex condition into a method and call that method in if condition. It would be much more readable and maintainable. e.g. Take a look at the example provided in the link.refactoring.guru/decompose-conditional
– Hardik Modha
yesterday
2
@HardikModha Yeah, I see how I might be able to implement that in the code I have now for modular clarity. Thank you.
– Andrew Truckle
yesterday
|
show 17 more comments
5
I would use a table instead of complexif
statement. Additionally, as these are boolean flags, you can model each scenario as a constant and check against it.
– Zdeslav Vojkovic
yesterday
3
if (!((bValue1 && bValue2 && bValue3) || (bValue1 && !bValue2 && !bValue3 && !bValue4)))
– mch
yesterday
8
what are the scenarios actually? Often things get much simpler if you just give stuff proper names, egbool scenario1 = bValue1 && bValue2 && bValue3 && bValue4;
– user463035818
yesterday
2
Using meaningful names, you can extract each complex condition into a method and call that method in if condition. It would be much more readable and maintainable. e.g. Take a look at the example provided in the link.refactoring.guru/decompose-conditional
– Hardik Modha
yesterday
2
@HardikModha Yeah, I see how I might be able to implement that in the code I have now for modular clarity. Thank you.
– Andrew Truckle
yesterday
5
5
I would use a table instead of complex
if
statement. Additionally, as these are boolean flags, you can model each scenario as a constant and check against it.– Zdeslav Vojkovic
yesterday
I would use a table instead of complex
if
statement. Additionally, as these are boolean flags, you can model each scenario as a constant and check against it.– Zdeslav Vojkovic
yesterday
3
3
if (!((bValue1 && bValue2 && bValue3) || (bValue1 && !bValue2 && !bValue3 && !bValue4)))
– mch
yesterday
if (!((bValue1 && bValue2 && bValue3) || (bValue1 && !bValue2 && !bValue3 && !bValue4)))
– mch
yesterday
8
8
what are the scenarios actually? Often things get much simpler if you just give stuff proper names, eg
bool scenario1 = bValue1 && bValue2 && bValue3 && bValue4;
– user463035818
yesterday
what are the scenarios actually? Often things get much simpler if you just give stuff proper names, eg
bool scenario1 = bValue1 && bValue2 && bValue3 && bValue4;
– user463035818
yesterday
2
2
Using meaningful names, you can extract each complex condition into a method and call that method in if condition. It would be much more readable and maintainable. e.g. Take a look at the example provided in the link.refactoring.guru/decompose-conditional
– Hardik Modha
yesterday
Using meaningful names, you can extract each complex condition into a method and call that method in if condition. It would be much more readable and maintainable. e.g. Take a look at the example provided in the link.refactoring.guru/decompose-conditional
– Hardik Modha
yesterday
2
2
@HardikModha Yeah, I see how I might be able to implement that in the code I have now for modular clarity. Thank you.
– Andrew Truckle
yesterday
@HardikModha Yeah, I see how I might be able to implement that in the code I have now for modular clarity. Thank you.
– Andrew Truckle
yesterday
|
show 17 more comments
24 Answers
24
active
oldest
votes
up vote
112
down vote
accepted
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, but in this case is ok: easy and readable.
If your logic gets more complicated, consider using something more to store different available scenarios (as Zladeck is suggesting).
Damn, simplicity is a virtue. I think this is the best answer, far better than mine or any other obfuscating technique! Bravo!
– gsamaras
yesterday
1
sure @hessamhedieh, it's ok only for a small number of available scenario. as I said, if things get more complicated, better look for something else
– Gian Paolo
yesterday
2
I'd've wrapped it in aif($bValue1)
as that always has to be true, technically allowing some minor performance improvement (though we're talking about negligable amounts here).
– Martijn
yesterday
1
41 (and counting) upvote for this (my) answer suggesting 3 ifs. I should reconsider my opinions about ifs :O
– Gian Paolo
yesterday
1
FWIW: there are only 2 scenarios: the first 2 are the same scenario and do not depend onbValue4
– Dancrumb
yesterday
|
show 12 more comments
up vote
57
down vote
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 thetrue
scenarios is good practice.
58
Though technically correct, this code requires a lot of comments in order to be edited by another developer few months later.
– Zdeslav Vojkovic
yesterday
9
@ZdeslavVojkovic: I would just add a comment with the equation.//!(ABC + AB'C'D') (By K-Map logic)
. That would be a good time for the developer to learn K-Maps if he doesn't already know them.
– P.W
yesterday
6
I agree with that, but IMO the problem is that it doesn't map clearly to the problem domain, i.e. how each condition maps to specific scenario which makes it hard to change/extend. What happens when there areE
andF
conditions and 4 new scenarios? How long it takes to update thisif
statement correctly? How does code review check if it is ok or not? The problem is not with the technical side but with "business" side.
– Zdeslav Vojkovic
yesterday
5
I think you can factor outA
:ABC + AB'C'D' = A(BC + B'C'D')
(this can be even factored toA(B ^ C)'(C + D')
though I'd be careful with calling this 'simplification').
– Maciej Piechotka
yesterday
13
@P.W That comment seems about as understandable as the code, and is thus a bit pointless. A better comment would explain how you actually came up with that equation, i.e. that the statement should trigger for TTTT, TTTF and TFFF. At that point you might as well just write those three conditions in the code instead and not need an explanation at all.
– Dukeling
yesterday
|
show 6 more comments
up vote
37
down vote
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");
}
3
Not the most maintainable but definitely simplifies the if condition. So leaving a few comments around the bitwise operations will be an absolute necessity here imo.
– Adam Zahran
yesterday
3
IMO, table is the best approach as it scales better with additional scenarios and flags.
– Zdeslav Vojkovic
yesterday
I like your first solution, easy to read and open to modification. I would make 2 improvements: 1: assign values to scenarioX with an explicit indication of boolean values used, e.g.SCENARIO_2 = true << 3 | true << 2 | true << 1 | false;
2: avoid SCENARIO_X variables and then store all available scenarios in a<std::set<int>
. Adding a scenario is going to be just something asmySet.insert( true << 3 | false << 2 | true << 1 | false;
maybe a little overkill for just 3 scenario, OP accepted the quick, dirty and easy solution I suggested in my answer.
– Gian Paolo
yesterday
2
If you're using C++14 or higher, I'd suggest instead using binary literals for the first solution - 0b1111, 0b1110 and 0b1000 is much clearer. You can probably also simplify this a bit using the standard library (std::find
?).
– Dukeling
yesterday
2
I find that binary literals here would be a minimal requirement to make the first code clean. In its current form it’s completely cryptic. Descriptive identifiers might help but I’m not even sure about that. In fact, the bit operations to produce thescenario
value strike me as unnecessarily error-prone.
– Konrad Rudolph
yesterday
add a comment |
up vote
16
down vote
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.
(Almost) off topic:
I don't write lot of answers here at StackOverflow. It's really funny that the above accepted anwser is the most appreciated answer in my history, granting me a couple of badges at SO.
And 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 :)
4
I'd consider removing the last 3 paragraphs as they add nothing to your answer. Consider editing your other answer to add the part about "simplicity is often / always the right way to do something. Readability/maintainability beats a few lines saved".
– Tas
yesterday
add a comment |
up vote
9
down vote
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
}
}
3
Thanks for sharing your alternative approach.
– Andrew Truckle
yesterday
1
I love this approach, except for the main function’s name: “from bool … to what?” — Why not explicitly,bitmap_from_bools
, orbools_to_bitmap
?
– Konrad Rudolph
yesterday
yes @KonradRudolph, I couldn't think of a better name, except maybebools_to_unsigned
. Bitmap is a good keyword; edited.
– Stack Danny
19 hours ago
add a comment |
up vote
8
down vote
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)) ...
Yes, but hard to understand. But thanks for suggestion.
– Andrew Truckle
yesterday
I compared compilers ability to simplify expression with your simplification as a reference: compiler explorer. gcc did not find your optimal version but its solution is still good. Clang and MSVC don't seem to perform any boolean expression simplification.
– Oliv
yesterday
1
@AndrewTruckle: note, that if you needed a more readable version, then please say so. You've said "simplified", yet you accept an even more verbose version than your original one.
– geza
yesterday
@Oliv: thanks for doing this test! This proves the point that while compilers are getting better and better, they still have a lot to learn :)
– geza
yesterday
1
simple
is indeed a vague term. Many people understand it in this context as simpler for developer to understand and not for the compiler to generate code, so more verbose can indeed be simpler.
– Zdeslav Vojkovic
yesterday
|
show 6 more comments
up vote
8
down vote
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 (scenari1 || 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 (scenari1or2 || scenario3) {
// Do whatever.
}
What's important here is not predicate logic or Karnaugh maps. It's describint 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.
1
+1 I am surprised there are 0 upvotes here. The solution is short, self-documenting (no comments needed), and easy to modify with little chance of introducing bugs. A clear favourite.
– RedFilter
11 hours ago
add a comment |
up vote
6
down vote
I'm not seeing any answers saying to name the scenarios, though the OP's solution does exactly that.
To me it is best to encapsulate the comment of what each scenario is into either a variable name or function name. You're more likely to ignore a comment than a name, and if your logic changes in the future you're more likely to change a name than a comment. You can't refactor a comment.
If you plan on reusing these scenarios outside of your function (or might want to), then make a function that says what it evaluates (constexpr
/noexcept
optional but recommended):
constexpr bool IsScenario1(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && b2 && b3 && b4; }
constexpr bool IsScenario2(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && b2 && b3 && !b4; }
constexpr bool IsScenario3(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && !b2 && !b3 && !b4; }
Make these class methods if possible (like in OP's solution). You can use variables inside of your function if you don't think you'll reuse the logic:
const auto is_scenario_1 = bValue1 && bValue2 && bValue3 && bValue4;
const auto is_scenario_2 = bvalue1 && bvalue2 && bValue3 && !bValue4;
const auto is_scenario_3 = bValue1 && !bValue2 && !bValue3 && !bValue4;
The compiler will most likely sort out that if bValue1 is false then all scenarios are false. Don't worry about making it fast, just correct and readable. If you profile your code and find this to be a bottleneck because the compiler generated sub-optimal code at -O2 or higher then try to rewrite it.
I like this slightly more than Gian Paolo's (already nice) solution: It avoids control flow and the use of a variable that is overwritten - more functional style.
– Dirk Herrmann
yesterday
add a comment |
up vote
5
down vote
A C/C++ way
bool scenario[3][4] = {{true, true, true, true},
{true, true, true, false},
{true, false, false, false}};
bool CheckScenario(bool bValue1, bool bValue2, bool bValue3, bool bValue4)
{
bool temp = {bValue1, bValue2, bValue3, bValue4};
for(int i = 0 ; i < sizeof(scenario) / sizeof(scenario[0]); i++)
{
if(memcmp(temp, scenario[i], sizeof(temp)) == 0)
return true;
}
return false;
}
This approach is scalable as if the number of valid conditions grow, you easily just add more of them to scenario list.
Thank you for your answer.
– Andrew Truckle
yesterday
I'm pretty sure this is wrong, though. It assumes that the compiler uses only a single binary representation fortrue
. A compiler which uses "anything non-zero is true" causes this code to fail. Note thattrue
must convert to1
, it just doesn't need to be stored as such.
– MSalters
yesterday
@MSalters, tnx, I get your point and I am aware of that, kinda like2 is not equal to true but evaluates to true
, my code doesnt forceint 1 = true
and works as long as all true's are converted to same int value, SO here is my question: Why compiler should act random on converting true to underlying int, Can you please elaborate more?
– hessam hedieh
yesterday
Performing amemcmp
to test boolean conditions is not the C++ way, and I rather doubt that it’s an established C way, either.
– Konrad Rudolph
yesterday
@hessamhedieh: The problem in your logic is "converting true to underlying int". That is not how compilers work,
– MSalters
16 hours ago
add a comment |
up vote
5
down vote
I am only providing my answer here as in the comments someone suggested to show my solution. I want to thank everyone for their insights.
In the end I opted to add three new "scenario" boolean
methods:
bool CChristianLifeMinistryValidationDlg::IsFirstWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
return (INCLUDE_ITEM1(pEntry) &&
!INCLUDE_ITEM2(pEntry) &&
!INCLUDE_ITEM3(pEntry) &&
!INCLUDE_ITEM4(pEntry));
}
bool CChristianLifeMinistryValidationDlg::IsSecondWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
return (INCLUDE_ITEM1(pEntry) &&
INCLUDE_ITEM2(pEntry) &&
INCLUDE_ITEM3(pEntry) &&
INCLUDE_ITEM4(pEntry));
}
bool CChristianLifeMinistryValidationDlg::IsOtherWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
return (INCLUDE_ITEM1(pEntry) &&
INCLUDE_ITEM2(pEntry) &&
INCLUDE_ITEM3(pEntry) &&
!INCLUDE_ITEM4(pEntry));
}
Then I was able to apply those my my validation routine like this:
if (!IsFirstWeekStudentItems(pEntry) && !IsSecondWeekStudentItems(pEntry) && !IsOtherWeekStudentItems(pEntry))
{
; Error
}
In my live application the 4 bool values are actually extracted from a DWORD
which has 4 values encoded into it.
Thanks again everyone.
1
Thanks for sharing the solution. :) It's actually better than the complex if conditions hell. Maybe you can still nameINCLUDE_ITEM1
etc in a better way and you are all good. :)
– Hardik Modha
yesterday
1
@HardikModha Well, technically they are "Student items" and the flag is to indicate if they are to be "included". So I think the name, albeit sounding generic, is actually meaningful in this context. :)
– Andrew Truckle
yesterday
Well, Sounds good then. :)
– Hardik Modha
yesterday
add a comment |
up vote
4
down vote
It's easy to notice that first two scenarios are similar - they share most of the conditions. If you want to select in which scenario you are at the moment, you could write it like this (it's a modified @gian-paolo's solution):
bool valid = false;
if(bValue1 && bValue2 && bValue3)
{
if (bValue4)
valid = true; //scenario 1
else if (!bValue4)
valid = true; //scenario 2
}
else if (bValue1 && !bValue2 && !bValue3 && !bValue4)
valid = true; //scenario 3
Going further, you can notice, that first boolean needs to be always true, which is an entry condition, so you can end up with:
bool valid = false;
if(bValue1)
{
if(bValue2 && bValue3)
{
if (bValue4)
valid = true; //scenario 1
else if (!bValue4)
valid = true; //scenario 2
}
else if (!bValue2 && !bValue3 && !bValue4)
valid = true; //scenario 3
}
Even more, you can now clearly see, that bValue2 and bValue3 are somewhat connected - you could extract their state to some external functions or variables with more appropriate name (this is not always easy or appropriate though):
bool valid = false;
if(bValue1)
{
bool bValue1and2 = bValue1 && bValue2;
bool notBValue1and2 = !bValue2 && !bValue3;
if(bValue1and2)
{
if (bValue4)
valid = true; //scenario 1
else if (!bValue4)
valid = true; //scenario 2
}
else if (notBValue1and2 && !bValue4)
valid = true; //scenario 3
}
Doing it this way have some advantages and disadvantages:
- conditions are smaller, so it's easier to reason about them,
- it's easier to do nice renaming to make these conditions more understandable,
- but, they require to understand the scope,
- moreover it's more rigid
If you predict that there will be changes to the above logic, you should use more straightforward approach as presented by @gian-paolo.
Otherwise, if these conditions are well established, and are kind of "solid rules" that will never change, consider my last code snippet.
add a comment |
up vote
3
down vote
I would also use shortcut variables for clarity. As noted earlier scenario 1 equals to scenario 2, because the value of bValue4 doesn't influence the truth of those two scenarios.
bool MAJORLY_TRUE=bValue1 && bValue2 && bValue3
bool MAJORLY_FALSE=!(bValue2 || bValue3 || bValue4)
then your expression beomes:
if (MAJORLY_TRUE || (bValue1 && MAJORLY_FALSE))
{
// do something
}
else
{
// There is some error
}
Giving meaningful names to MAJORTRUE and MAJORFALSE variables (as well as actually to bValue* vars) would help a lot with readability and maintenance.
add a comment |
up vote
3
down vote
As suggested by mch, you could do:
if(!((bValue1 && bValue2 && bValue3) ||
(bValue1 && !bValue2 && !bValue3 && !bValue4))
)
where the first line covers the two first good cases, and the second line covers the last one.
Live Demo, where I played around and it passes your cases.
add a comment |
up vote
3
down vote
Focus on readability of the problem, not the specific "if" statement.
While this will produce more lines of code, and some may consider it either overkill or unnecessary. I'd suggest that abstracting your scenarios from the specific booleans is the best way to maintain readability.
By splitting things into classes (feel free to just use functions, or whatever other tool you prefer) with understandable names - we can much more easily show the meanings behind each scenario. More importantly, in a system with many moving parts - it is easier to maintain and join into your existing systems (again, despite how much extra code is involed).
#include <iostream>
#include <vector>
using namespace std;
// These values would likely not come from a single struct in real life
// Instead, they may be references to other booleans in other systems
struct Values
{
bool bValue1; // These would be given better names in reality
bool bValue2; // e.g. bDidTheCarCatchFire
bool bValue3; // and bDidTheWindshieldFallOff
bool bValue4;
};
class Scenario
{
public:
Scenario(Values& values)
: mValues(values) {}
virtual operator bool() = 0;
protected:
Values& mValues;
};
// Names as examples of things that describe your "scenarios" more effectively
class Scenario1_TheCarWasNotDamagedAtAll : public Scenario
{
public:
Scenario1_TheCarWasNotDamagedAtAll(Values& values) : Scenario(values) {}
virtual operator bool()
{
return mValues.bValue1
&& mValues.bValue2
&& mValues.bValue3
&& mValues.bValue4;
}
};
class Scenario2_TheCarBreaksDownButDidntGoOnFire : public Scenario
{
public:
Scenario2_TheCarBreaksDownButDidntGoOnFire(Values& values) : Scenario(values) {}
virtual operator bool()
{
return mValues.bValue1
&& mValues.bValue2
&& mValues.bValue3
&& !mValues.bValue4;
}
};
class Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere : public Scenario
{
public:
Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere(Values& values) : Scenario(values) {}
virtual operator bool()
{
return mValues.bValue1
&& !mValues.bValue2
&& !mValues.bValue3
&& !mValues.bValue4;
}
};
Scenario* findMatchingScenario(std::vector<Scenario*>& scenarios)
{
for(std::vector<Scenario*>::iterator it = scenarios.begin(); it != scenarios.end(); it++)
{
if (**it)
{
return *it;
}
}
return NULL;
}
int main() {
Values values = {true, true, true, true};
std::vector<Scenario*> scenarios = {
new Scenario1_TheCarWasNotDamagedAtAll(values),
new Scenario2_TheCarBreaksDownButDidntGoOnFire(values),
new Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere(values)
};
Scenario* matchingScenario = findMatchingScenario(scenarios);
if(matchingScenario)
{
std::cout << matchingScenario << " was a match" << std::endl;
}
else
{
std::cout << "No match" << std::endl;
}
// your code goes here
return 0;
}
5
At some point, verbosity starts to harm readability. I think this goes too far.
– JollyJoker
yesterday
1
@JollyJoker I do actually agree in this specific situation - however, my gut feeling from the way OP has named everything extremely generically, is that their "real" code is likely a lot more complex than the example they've given. Really, I just wanted to put this alternative out there, as it's how I'd structure it for something far more complex/involved. But you're right - for OPs specific example, it is overly verbose and makes matters worse.
– Bilkokuya
yesterday
add a comment |
up vote
2
down vote
I am denoting a, b, c, d for clarity, and A, B, C, D for complements
bValue1 = a (!A)
bValue2 = b (!B)
bValue3 = c (!C)
bValue4 = d (!D)
Equation
1 = abcd + abcD + aBCD
= a (bcd + bcD + BCD)
= a (bc + BCD)
= a (bcd + D (b ^C))
Use any equations that suits you.
add a comment |
up vote
2
down vote
A slight variation on @GianPaolo's fine answer, which some may find easier to read:
bool any_of_three_scenarios(bool v1, bool v2, bool v3, bool v4)
{
return (v1 && v2 && v3 && v4) // scenario 1
|| (v1 && v2 && v3 && !v4) // scenario 2
|| (v1 && !v2 && !v3 && !v4); // scenario 3
}
if (any_of_three_scenarios(bValue1,bValue2,bValue3,bValue4))
{
// ...
}
add a comment |
up vote
2
down vote
Consider translating your tables as directly as possible into your program. Drive the program based off the table, instead of mimicing it with logic.
template<class T0>
auto is_any_of( T0 const& t0, std::initializer_list<T0> il ) {
for (auto&& x:il)
if (x==t0) return true;
return false;
}
now
if (is_any_of(
std::make_tuple(bValue1, bValue2, bValue3, bValue4),
{
{true, true, true, true},
{true, true, true, false},
{true, false, false, false}
}
))
this directly as possible encodes your truth table into the compiler.
Live example.
You could also use std::any_of
directly:
using entry = std::array<bool, 4>;
constexpr entry acceptable =
{
{true, true, true, true},
{true, true, true, false},
{true, false, false, false}
};
if (std::any_of( begin(acceptable), end(acceptable), [&](auto&&x){
return entry{bValue1, bValue2, bValue3, bValue4} == x;
}) {
}
the compiler can inline the code, and eliminate any iteration and build its own logic for you. Meanwhile, your code reflects exactly how you concieved of the problem.
The first version is so easy to read and so maintenable, I really like it. The second one is harder to read, at least for me, and requires a c++ skill level maybe over the average, surely over my one. Not something everyone is able to write. Just learned somethin new, thanks
– Gian Paolo
5 hours ago
add a comment |
up vote
1
down vote
Every answer is overly complex and difficult to read. The best solution to this is a switch()
statement. It is both readable and makes adding/modifying additional simple. Compilers are good at optimising switch()
statements too.
switch( (bValue4 << 3) | (bValue3 << 2) | (bValue2 << 1) | (bValue1) )
{
case 0b1111:
// scenario 1
break;
case 0b0111:
// scenario 2
break;
case 0b0001:
// scenario 3
break;
default:
// fault condition
break;
}
You can of course use constants and OR them together in the case
statements for even greater readability.
add a comment |
up vote
1
down vote
First, assuming you can only modify the scenario check, I would focus on readability and just wrap the check in a function so that you can just call if(ScenarioA())
.
Now, assuming you actually want/need to optimize this, I would recommend converting the tightly linked Booleans into constant integers, and using bit operators on them
public class Options {
public const bool A = 2; // 0001
public const bool B = 4; // 0010
public const bool C = 16;// 0100
public const bool D = 32;// 1000
//public const bool N = 2^n; (up to n=32)
}
...
public isScenario3(int options) {
int s3 = Options.A | Options.B | Options.C;
// for true if only s3 options are set
return options == s3;
// for true if s3 options are set
// return options & s3 == s3
}
This makes expressing the scenarios as easy as listing what is part of it, allows you to use a switch statement to jump to the right condition, and confuse fellow developers who have not seen this before. (C# RegexOptions uses this pattern for setting flags, I don't know if there is a c++ library example)
add a comment |
up vote
1
down vote
It depends on what they represent.
For example if 1 is a key, and 2 and 3 are two people who must agree (except if they agree on NOT
they need a third person - 4 - to confirm) the most readable might be:
1 &&
(
(2 && 3)
||
((!2 && !3) && !4)
)
1
You might be right, but using numbers to illustrate your point detracts from your answer. Try using descriptive names.
– jxh
yesterday
1
@jxh Those are the numbers OP used. I just removed thebValue
.
– ispiro
yesterday
Left as generic booleans, your construction does not improve readability. Readability only improves if the booleans are given descriptive names that motivates the construction you suggest.
– jxh
yesterday
add a comment |
up vote
1
down vote
If (!bValue1 || (bValue2 != bValue3) || (!bValue4 && bValue2))
{
// you have a problem
}
- b1 must always be true
- b2 must always equal b3
- and b4 cannot be false
if b2 (and b3) are true
simple
New contributor
add a comment |
up vote
1
down vote
Doing bitwise operation looks very clean and understandable.
int bitwise = (bValue4 << 3) | (bValue3 << 2) | (bValue2 << 1) | (bValue1);
if (bitwise == 0b1111 || bitwise == 0b0111 || bitwise == 0b0001)
{
//satisfying condition
}
add a comment |
up vote
-2
down vote
use bit field:
unoin {
struct {
bool b1: 1;
bool b2: 1;
bool b3: 1;
bool b4: 1;
} b;
int i;
} u;
// set:
u.b.b1=true;
...
// test
if (u.i == 0x0f) {...}
if (u.i == 0x0e) {...}
if (u.i == 0x08) {...}
2
This causes UB due to accessing an inactive union field.
– HolyBlackCat
13 hours ago
Formally it's UB in C++, you can't set one member of union and read from another. Technically it might be better to implement templated getterssetters for bits of integral value.
– Swift - Friday Pie
13 hours ago
I think the behavior would shift to Implementation-Defined if one were to convert the union's address to anunsigned char*
, though I think simply using something like((((flag4 <<1) | flag3) << 1) | flag2) << 1) | flag1
would probably be more efficient.
– supercat
7 hours ago
add a comment |
up vote
-2
down vote
A simple approach is finding the answer that you think are acceptable.
Yes = (boolean1 && boolean2 && boolean3 && boolean4) + + ...
Now if possible simplify the equation using boolean algebra.
like in this case, acceptable1 and 2 combine to (boolean1 && boolean2 && boolean3)
Hence the final answer is
(boolean1 && boolean2 && boolean3) + ((boolean1 && !boolean2 && !boolean3 && !boolean4)
P.S. + means OR operator.
add a comment |
24 Answers
24
active
oldest
votes
24 Answers
24
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
112
down vote
accepted
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, but in this case is ok: easy and readable.
If your logic gets more complicated, consider using something more to store different available scenarios (as Zladeck is suggesting).
Damn, simplicity is a virtue. I think this is the best answer, far better than mine or any other obfuscating technique! Bravo!
– gsamaras
yesterday
1
sure @hessamhedieh, it's ok only for a small number of available scenario. as I said, if things get more complicated, better look for something else
– Gian Paolo
yesterday
2
I'd've wrapped it in aif($bValue1)
as that always has to be true, technically allowing some minor performance improvement (though we're talking about negligable amounts here).
– Martijn
yesterday
1
41 (and counting) upvote for this (my) answer suggesting 3 ifs. I should reconsider my opinions about ifs :O
– Gian Paolo
yesterday
1
FWIW: there are only 2 scenarios: the first 2 are the same scenario and do not depend onbValue4
– Dancrumb
yesterday
|
show 12 more comments
up vote
112
down vote
accepted
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, but in this case is ok: easy and readable.
If your logic gets more complicated, consider using something more to store different available scenarios (as Zladeck is suggesting).
Damn, simplicity is a virtue. I think this is the best answer, far better than mine or any other obfuscating technique! Bravo!
– gsamaras
yesterday
1
sure @hessamhedieh, it's ok only for a small number of available scenario. as I said, if things get more complicated, better look for something else
– Gian Paolo
yesterday
2
I'd've wrapped it in aif($bValue1)
as that always has to be true, technically allowing some minor performance improvement (though we're talking about negligable amounts here).
– Martijn
yesterday
1
41 (and counting) upvote for this (my) answer suggesting 3 ifs. I should reconsider my opinions about ifs :O
– Gian Paolo
yesterday
1
FWIW: there are only 2 scenarios: the first 2 are the same scenario and do not depend onbValue4
– Dancrumb
yesterday
|
show 12 more comments
up vote
112
down vote
accepted
up vote
112
down vote
accepted
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, but in this case is ok: easy and readable.
If your logic gets more complicated, consider using something more to store different available scenarios (as Zladeck is suggesting).
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, but in this case is ok: easy and readable.
If your logic gets more complicated, consider using something more to store different available scenarios (as Zladeck is suggesting).
edited yesterday
Andrew Truckle
5,04732145
5,04732145
answered yesterday
Gian Paolo
2,5432925
2,5432925
Damn, simplicity is a virtue. I think this is the best answer, far better than mine or any other obfuscating technique! Bravo!
– gsamaras
yesterday
1
sure @hessamhedieh, it's ok only for a small number of available scenario. as I said, if things get more complicated, better look for something else
– Gian Paolo
yesterday
2
I'd've wrapped it in aif($bValue1)
as that always has to be true, technically allowing some minor performance improvement (though we're talking about negligable amounts here).
– Martijn
yesterday
1
41 (and counting) upvote for this (my) answer suggesting 3 ifs. I should reconsider my opinions about ifs :O
– Gian Paolo
yesterday
1
FWIW: there are only 2 scenarios: the first 2 are the same scenario and do not depend onbValue4
– Dancrumb
yesterday
|
show 12 more comments
Damn, simplicity is a virtue. I think this is the best answer, far better than mine or any other obfuscating technique! Bravo!
– gsamaras
yesterday
1
sure @hessamhedieh, it's ok only for a small number of available scenario. as I said, if things get more complicated, better look for something else
– Gian Paolo
yesterday
2
I'd've wrapped it in aif($bValue1)
as that always has to be true, technically allowing some minor performance improvement (though we're talking about negligable amounts here).
– Martijn
yesterday
1
41 (and counting) upvote for this (my) answer suggesting 3 ifs. I should reconsider my opinions about ifs :O
– Gian Paolo
yesterday
1
FWIW: there are only 2 scenarios: the first 2 are the same scenario and do not depend onbValue4
– Dancrumb
yesterday
Damn, simplicity is a virtue. I think this is the best answer, far better than mine or any other obfuscating technique! Bravo!
– gsamaras
yesterday
Damn, simplicity is a virtue. I think this is the best answer, far better than mine or any other obfuscating technique! Bravo!
– gsamaras
yesterday
1
1
sure @hessamhedieh, it's ok only for a small number of available scenario. as I said, if things get more complicated, better look for something else
– Gian Paolo
yesterday
sure @hessamhedieh, it's ok only for a small number of available scenario. as I said, if things get more complicated, better look for something else
– Gian Paolo
yesterday
2
2
I'd've wrapped it in a
if($bValue1)
as that always has to be true, technically allowing some minor performance improvement (though we're talking about negligable amounts here).– Martijn
yesterday
I'd've wrapped it in a
if($bValue1)
as that always has to be true, technically allowing some minor performance improvement (though we're talking about negligable amounts here).– Martijn
yesterday
1
1
41 (and counting) upvote for this (my) answer suggesting 3 ifs. I should reconsider my opinions about ifs :O
– Gian Paolo
yesterday
41 (and counting) upvote for this (my) answer suggesting 3 ifs. I should reconsider my opinions about ifs :O
– Gian Paolo
yesterday
1
1
FWIW: there are only 2 scenarios: the first 2 are the same scenario and do not depend on
bValue4
– Dancrumb
yesterday
FWIW: there are only 2 scenarios: the first 2 are the same scenario and do not depend on
bValue4
– Dancrumb
yesterday
|
show 12 more comments
up vote
57
down vote
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 thetrue
scenarios is good practice.
58
Though technically correct, this code requires a lot of comments in order to be edited by another developer few months later.
– Zdeslav Vojkovic
yesterday
9
@ZdeslavVojkovic: I would just add a comment with the equation.//!(ABC + AB'C'D') (By K-Map logic)
. That would be a good time for the developer to learn K-Maps if he doesn't already know them.
– P.W
yesterday
6
I agree with that, but IMO the problem is that it doesn't map clearly to the problem domain, i.e. how each condition maps to specific scenario which makes it hard to change/extend. What happens when there areE
andF
conditions and 4 new scenarios? How long it takes to update thisif
statement correctly? How does code review check if it is ok or not? The problem is not with the technical side but with "business" side.
– Zdeslav Vojkovic
yesterday
5
I think you can factor outA
:ABC + AB'C'D' = A(BC + B'C'D')
(this can be even factored toA(B ^ C)'(C + D')
though I'd be careful with calling this 'simplification').
– Maciej Piechotka
yesterday
13
@P.W That comment seems about as understandable as the code, and is thus a bit pointless. A better comment would explain how you actually came up with that equation, i.e. that the statement should trigger for TTTT, TTTF and TFFF. At that point you might as well just write those three conditions in the code instead and not need an explanation at all.
– Dukeling
yesterday
|
show 6 more comments
up vote
57
down vote
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 thetrue
scenarios is good practice.
58
Though technically correct, this code requires a lot of comments in order to be edited by another developer few months later.
– Zdeslav Vojkovic
yesterday
9
@ZdeslavVojkovic: I would just add a comment with the equation.//!(ABC + AB'C'D') (By K-Map logic)
. That would be a good time for the developer to learn K-Maps if he doesn't already know them.
– P.W
yesterday
6
I agree with that, but IMO the problem is that it doesn't map clearly to the problem domain, i.e. how each condition maps to specific scenario which makes it hard to change/extend. What happens when there areE
andF
conditions and 4 new scenarios? How long it takes to update thisif
statement correctly? How does code review check if it is ok or not? The problem is not with the technical side but with "business" side.
– Zdeslav Vojkovic
yesterday
5
I think you can factor outA
:ABC + AB'C'D' = A(BC + B'C'D')
(this can be even factored toA(B ^ C)'(C + D')
though I'd be careful with calling this 'simplification').
– Maciej Piechotka
yesterday
13
@P.W That comment seems about as understandable as the code, and is thus a bit pointless. A better comment would explain how you actually came up with that equation, i.e. that the statement should trigger for TTTT, TTTF and TFFF. At that point you might as well just write those three conditions in the code instead and not need an explanation at all.
– Dukeling
yesterday
|
show 6 more comments
up vote
57
down vote
up vote
57
down vote
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 thetrue
scenarios is good practice.
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 thetrue
scenarios is good practice.
edited 21 hours ago
answered yesterday
P.W
9,1732742
9,1732742
58
Though technically correct, this code requires a lot of comments in order to be edited by another developer few months later.
– Zdeslav Vojkovic
yesterday
9
@ZdeslavVojkovic: I would just add a comment with the equation.//!(ABC + AB'C'D') (By K-Map logic)
. That would be a good time for the developer to learn K-Maps if he doesn't already know them.
– P.W
yesterday
6
I agree with that, but IMO the problem is that it doesn't map clearly to the problem domain, i.e. how each condition maps to specific scenario which makes it hard to change/extend. What happens when there areE
andF
conditions and 4 new scenarios? How long it takes to update thisif
statement correctly? How does code review check if it is ok or not? The problem is not with the technical side but with "business" side.
– Zdeslav Vojkovic
yesterday
5
I think you can factor outA
:ABC + AB'C'D' = A(BC + B'C'D')
(this can be even factored toA(B ^ C)'(C + D')
though I'd be careful with calling this 'simplification').
– Maciej Piechotka
yesterday
13
@P.W That comment seems about as understandable as the code, and is thus a bit pointless. A better comment would explain how you actually came up with that equation, i.e. that the statement should trigger for TTTT, TTTF and TFFF. At that point you might as well just write those three conditions in the code instead and not need an explanation at all.
– Dukeling
yesterday
|
show 6 more comments
58
Though technically correct, this code requires a lot of comments in order to be edited by another developer few months later.
– Zdeslav Vojkovic
yesterday
9
@ZdeslavVojkovic: I would just add a comment with the equation.//!(ABC + AB'C'D') (By K-Map logic)
. That would be a good time for the developer to learn K-Maps if he doesn't already know them.
– P.W
yesterday
6
I agree with that, but IMO the problem is that it doesn't map clearly to the problem domain, i.e. how each condition maps to specific scenario which makes it hard to change/extend. What happens when there areE
andF
conditions and 4 new scenarios? How long it takes to update thisif
statement correctly? How does code review check if it is ok or not? The problem is not with the technical side but with "business" side.
– Zdeslav Vojkovic
yesterday
5
I think you can factor outA
:ABC + AB'C'D' = A(BC + B'C'D')
(this can be even factored toA(B ^ C)'(C + D')
though I'd be careful with calling this 'simplification').
– Maciej Piechotka
yesterday
13
@P.W That comment seems about as understandable as the code, and is thus a bit pointless. A better comment would explain how you actually came up with that equation, i.e. that the statement should trigger for TTTT, TTTF and TFFF. At that point you might as well just write those three conditions in the code instead and not need an explanation at all.
– Dukeling
yesterday
58
58
Though technically correct, this code requires a lot of comments in order to be edited by another developer few months later.
– Zdeslav Vojkovic
yesterday
Though technically correct, this code requires a lot of comments in order to be edited by another developer few months later.
– Zdeslav Vojkovic
yesterday
9
9
@ZdeslavVojkovic: I would just add a comment with the equation.
//!(ABC + AB'C'D') (By K-Map logic)
. That would be a good time for the developer to learn K-Maps if he doesn't already know them.– P.W
yesterday
@ZdeslavVojkovic: I would just add a comment with the equation.
//!(ABC + AB'C'D') (By K-Map logic)
. That would be a good time for the developer to learn K-Maps if he doesn't already know them.– P.W
yesterday
6
6
I agree with that, but IMO the problem is that it doesn't map clearly to the problem domain, i.e. how each condition maps to specific scenario which makes it hard to change/extend. What happens when there are
E
and F
conditions and 4 new scenarios? How long it takes to update this if
statement correctly? How does code review check if it is ok or not? The problem is not with the technical side but with "business" side.– Zdeslav Vojkovic
yesterday
I agree with that, but IMO the problem is that it doesn't map clearly to the problem domain, i.e. how each condition maps to specific scenario which makes it hard to change/extend. What happens when there are
E
and F
conditions and 4 new scenarios? How long it takes to update this if
statement correctly? How does code review check if it is ok or not? The problem is not with the technical side but with "business" side.– Zdeslav Vojkovic
yesterday
5
5
I think you can factor out
A
: ABC + AB'C'D' = A(BC + B'C'D')
(this can be even factored to A(B ^ C)'(C + D')
though I'd be careful with calling this 'simplification').– Maciej Piechotka
yesterday
I think you can factor out
A
: ABC + AB'C'D' = A(BC + B'C'D')
(this can be even factored to A(B ^ C)'(C + D')
though I'd be careful with calling this 'simplification').– Maciej Piechotka
yesterday
13
13
@P.W That comment seems about as understandable as the code, and is thus a bit pointless. A better comment would explain how you actually came up with that equation, i.e. that the statement should trigger for TTTT, TTTF and TFFF. At that point you might as well just write those three conditions in the code instead and not need an explanation at all.
– Dukeling
yesterday
@P.W That comment seems about as understandable as the code, and is thus a bit pointless. A better comment would explain how you actually came up with that equation, i.e. that the statement should trigger for TTTT, TTTF and TFFF. At that point you might as well just write those three conditions in the code instead and not need an explanation at all.
– Dukeling
yesterday
|
show 6 more comments
up vote
37
down vote
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");
}
3
Not the most maintainable but definitely simplifies the if condition. So leaving a few comments around the bitwise operations will be an absolute necessity here imo.
– Adam Zahran
yesterday
3
IMO, table is the best approach as it scales better with additional scenarios and flags.
– Zdeslav Vojkovic
yesterday
I like your first solution, easy to read and open to modification. I would make 2 improvements: 1: assign values to scenarioX with an explicit indication of boolean values used, e.g.SCENARIO_2 = true << 3 | true << 2 | true << 1 | false;
2: avoid SCENARIO_X variables and then store all available scenarios in a<std::set<int>
. Adding a scenario is going to be just something asmySet.insert( true << 3 | false << 2 | true << 1 | false;
maybe a little overkill for just 3 scenario, OP accepted the quick, dirty and easy solution I suggested in my answer.
– Gian Paolo
yesterday
2
If you're using C++14 or higher, I'd suggest instead using binary literals for the first solution - 0b1111, 0b1110 and 0b1000 is much clearer. You can probably also simplify this a bit using the standard library (std::find
?).
– Dukeling
yesterday
2
I find that binary literals here would be a minimal requirement to make the first code clean. In its current form it’s completely cryptic. Descriptive identifiers might help but I’m not even sure about that. In fact, the bit operations to produce thescenario
value strike me as unnecessarily error-prone.
– Konrad Rudolph
yesterday
add a comment |
up vote
37
down vote
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");
}
3
Not the most maintainable but definitely simplifies the if condition. So leaving a few comments around the bitwise operations will be an absolute necessity here imo.
– Adam Zahran
yesterday
3
IMO, table is the best approach as it scales better with additional scenarios and flags.
– Zdeslav Vojkovic
yesterday
I like your first solution, easy to read and open to modification. I would make 2 improvements: 1: assign values to scenarioX with an explicit indication of boolean values used, e.g.SCENARIO_2 = true << 3 | true << 2 | true << 1 | false;
2: avoid SCENARIO_X variables and then store all available scenarios in a<std::set<int>
. Adding a scenario is going to be just something asmySet.insert( true << 3 | false << 2 | true << 1 | false;
maybe a little overkill for just 3 scenario, OP accepted the quick, dirty and easy solution I suggested in my answer.
– Gian Paolo
yesterday
2
If you're using C++14 or higher, I'd suggest instead using binary literals for the first solution - 0b1111, 0b1110 and 0b1000 is much clearer. You can probably also simplify this a bit using the standard library (std::find
?).
– Dukeling
yesterday
2
I find that binary literals here would be a minimal requirement to make the first code clean. In its current form it’s completely cryptic. Descriptive identifiers might help but I’m not even sure about that. In fact, the bit operations to produce thescenario
value strike me as unnecessarily error-prone.
– Konrad Rudolph
yesterday
add a comment |
up vote
37
down vote
up vote
37
down vote
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");
}
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");
}
edited 17 hours ago
answered yesterday
Zdeslav Vojkovic
12.4k1836
12.4k1836
3
Not the most maintainable but definitely simplifies the if condition. So leaving a few comments around the bitwise operations will be an absolute necessity here imo.
– Adam Zahran
yesterday
3
IMO, table is the best approach as it scales better with additional scenarios and flags.
– Zdeslav Vojkovic
yesterday
I like your first solution, easy to read and open to modification. I would make 2 improvements: 1: assign values to scenarioX with an explicit indication of boolean values used, e.g.SCENARIO_2 = true << 3 | true << 2 | true << 1 | false;
2: avoid SCENARIO_X variables and then store all available scenarios in a<std::set<int>
. Adding a scenario is going to be just something asmySet.insert( true << 3 | false << 2 | true << 1 | false;
maybe a little overkill for just 3 scenario, OP accepted the quick, dirty and easy solution I suggested in my answer.
– Gian Paolo
yesterday
2
If you're using C++14 or higher, I'd suggest instead using binary literals for the first solution - 0b1111, 0b1110 and 0b1000 is much clearer. You can probably also simplify this a bit using the standard library (std::find
?).
– Dukeling
yesterday
2
I find that binary literals here would be a minimal requirement to make the first code clean. In its current form it’s completely cryptic. Descriptive identifiers might help but I’m not even sure about that. In fact, the bit operations to produce thescenario
value strike me as unnecessarily error-prone.
– Konrad Rudolph
yesterday
add a comment |
3
Not the most maintainable but definitely simplifies the if condition. So leaving a few comments around the bitwise operations will be an absolute necessity here imo.
– Adam Zahran
yesterday
3
IMO, table is the best approach as it scales better with additional scenarios and flags.
– Zdeslav Vojkovic
yesterday
I like your first solution, easy to read and open to modification. I would make 2 improvements: 1: assign values to scenarioX with an explicit indication of boolean values used, e.g.SCENARIO_2 = true << 3 | true << 2 | true << 1 | false;
2: avoid SCENARIO_X variables and then store all available scenarios in a<std::set<int>
. Adding a scenario is going to be just something asmySet.insert( true << 3 | false << 2 | true << 1 | false;
maybe a little overkill for just 3 scenario, OP accepted the quick, dirty and easy solution I suggested in my answer.
– Gian Paolo
yesterday
2
If you're using C++14 or higher, I'd suggest instead using binary literals for the first solution - 0b1111, 0b1110 and 0b1000 is much clearer. You can probably also simplify this a bit using the standard library (std::find
?).
– Dukeling
yesterday
2
I find that binary literals here would be a minimal requirement to make the first code clean. In its current form it’s completely cryptic. Descriptive identifiers might help but I’m not even sure about that. In fact, the bit operations to produce thescenario
value strike me as unnecessarily error-prone.
– Konrad Rudolph
yesterday
3
3
Not the most maintainable but definitely simplifies the if condition. So leaving a few comments around the bitwise operations will be an absolute necessity here imo.
– Adam Zahran
yesterday
Not the most maintainable but definitely simplifies the if condition. So leaving a few comments around the bitwise operations will be an absolute necessity here imo.
– Adam Zahran
yesterday
3
3
IMO, table is the best approach as it scales better with additional scenarios and flags.
– Zdeslav Vojkovic
yesterday
IMO, table is the best approach as it scales better with additional scenarios and flags.
– Zdeslav Vojkovic
yesterday
I like your first solution, easy to read and open to modification. I would make 2 improvements: 1: assign values to scenarioX with an explicit indication of boolean values used, e.g.
SCENARIO_2 = true << 3 | true << 2 | true << 1 | false;
2: avoid SCENARIO_X variables and then store all available scenarios in a <std::set<int>
. Adding a scenario is going to be just something as mySet.insert( true << 3 | false << 2 | true << 1 | false;
maybe a little overkill for just 3 scenario, OP accepted the quick, dirty and easy solution I suggested in my answer.– Gian Paolo
yesterday
I like your first solution, easy to read and open to modification. I would make 2 improvements: 1: assign values to scenarioX with an explicit indication of boolean values used, e.g.
SCENARIO_2 = true << 3 | true << 2 | true << 1 | false;
2: avoid SCENARIO_X variables and then store all available scenarios in a <std::set<int>
. Adding a scenario is going to be just something as mySet.insert( true << 3 | false << 2 | true << 1 | false;
maybe a little overkill for just 3 scenario, OP accepted the quick, dirty and easy solution I suggested in my answer.– Gian Paolo
yesterday
2
2
If you're using C++14 or higher, I'd suggest instead using binary literals for the first solution - 0b1111, 0b1110 and 0b1000 is much clearer. You can probably also simplify this a bit using the standard library (
std::find
?).– Dukeling
yesterday
If you're using C++14 or higher, I'd suggest instead using binary literals for the first solution - 0b1111, 0b1110 and 0b1000 is much clearer. You can probably also simplify this a bit using the standard library (
std::find
?).– Dukeling
yesterday
2
2
I find that binary literals here would be a minimal requirement to make the first code clean. In its current form it’s completely cryptic. Descriptive identifiers might help but I’m not even sure about that. In fact, the bit operations to produce the
scenario
value strike me as unnecessarily error-prone.– Konrad Rudolph
yesterday
I find that binary literals here would be a minimal requirement to make the first code clean. In its current form it’s completely cryptic. Descriptive identifiers might help but I’m not even sure about that. In fact, the bit operations to produce the
scenario
value strike me as unnecessarily error-prone.– Konrad Rudolph
yesterday
add a comment |
up vote
16
down vote
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.
(Almost) off topic:
I don't write lot of answers here at StackOverflow. It's really funny that the above accepted anwser is the most appreciated answer in my history, granting me a couple of badges at SO.
And 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 :)
4
I'd consider removing the last 3 paragraphs as they add nothing to your answer. Consider editing your other answer to add the part about "simplicity is often / always the right way to do something. Readability/maintainability beats a few lines saved".
– Tas
yesterday
add a comment |
up vote
16
down vote
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.
(Almost) off topic:
I don't write lot of answers here at StackOverflow. It's really funny that the above accepted anwser is the most appreciated answer in my history, granting me a couple of badges at SO.
And 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 :)
4
I'd consider removing the last 3 paragraphs as they add nothing to your answer. Consider editing your other answer to add the part about "simplicity is often / always the right way to do something. Readability/maintainability beats a few lines saved".
– Tas
yesterday
add a comment |
up vote
16
down vote
up vote
16
down vote
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.
(Almost) off topic:
I don't write lot of answers here at StackOverflow. It's really funny that the above accepted anwser is the most appreciated answer in my history, granting me a couple of badges at SO.
And 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 :)
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.
(Almost) off topic:
I don't write lot of answers here at StackOverflow. It's really funny that the above accepted anwser is the most appreciated answer in my history, granting me a couple of badges at SO.
And 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 :)
edited yesterday
answered yesterday
Gian Paolo
2,5432925
2,5432925
4
I'd consider removing the last 3 paragraphs as they add nothing to your answer. Consider editing your other answer to add the part about "simplicity is often / always the right way to do something. Readability/maintainability beats a few lines saved".
– Tas
yesterday
add a comment |
4
I'd consider removing the last 3 paragraphs as they add nothing to your answer. Consider editing your other answer to add the part about "simplicity is often / always the right way to do something. Readability/maintainability beats a few lines saved".
– Tas
yesterday
4
4
I'd consider removing the last 3 paragraphs as they add nothing to your answer. Consider editing your other answer to add the part about "simplicity is often / always the right way to do something. Readability/maintainability beats a few lines saved".
– Tas
yesterday
I'd consider removing the last 3 paragraphs as they add nothing to your answer. Consider editing your other answer to add the part about "simplicity is often / always the right way to do something. Readability/maintainability beats a few lines saved".
– Tas
yesterday
add a comment |
up vote
9
down vote
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
}
}
3
Thanks for sharing your alternative approach.
– Andrew Truckle
yesterday
1
I love this approach, except for the main function’s name: “from bool … to what?” — Why not explicitly,bitmap_from_bools
, orbools_to_bitmap
?
– Konrad Rudolph
yesterday
yes @KonradRudolph, I couldn't think of a better name, except maybebools_to_unsigned
. Bitmap is a good keyword; edited.
– Stack Danny
19 hours ago
add a comment |
up vote
9
down vote
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
}
}
3
Thanks for sharing your alternative approach.
– Andrew Truckle
yesterday
1
I love this approach, except for the main function’s name: “from bool … to what?” — Why not explicitly,bitmap_from_bools
, orbools_to_bitmap
?
– Konrad Rudolph
yesterday
yes @KonradRudolph, I couldn't think of a better name, except maybebools_to_unsigned
. Bitmap is a good keyword; edited.
– Stack Danny
19 hours ago
add a comment |
up vote
9
down vote
up vote
9
down vote
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
}
}
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
}
}
edited 19 hours ago
answered yesterday
Stack Danny
929318
929318
3
Thanks for sharing your alternative approach.
– Andrew Truckle
yesterday
1
I love this approach, except for the main function’s name: “from bool … to what?” — Why not explicitly,bitmap_from_bools
, orbools_to_bitmap
?
– Konrad Rudolph
yesterday
yes @KonradRudolph, I couldn't think of a better name, except maybebools_to_unsigned
. Bitmap is a good keyword; edited.
– Stack Danny
19 hours ago
add a comment |
3
Thanks for sharing your alternative approach.
– Andrew Truckle
yesterday
1
I love this approach, except for the main function’s name: “from bool … to what?” — Why not explicitly,bitmap_from_bools
, orbools_to_bitmap
?
– Konrad Rudolph
yesterday
yes @KonradRudolph, I couldn't think of a better name, except maybebools_to_unsigned
. Bitmap is a good keyword; edited.
– Stack Danny
19 hours ago
3
3
Thanks for sharing your alternative approach.
– Andrew Truckle
yesterday
Thanks for sharing your alternative approach.
– Andrew Truckle
yesterday
1
1
I love this approach, except for the main function’s name: “from bool … to what?” — Why not explicitly,
bitmap_from_bools
, or bools_to_bitmap
?– Konrad Rudolph
yesterday
I love this approach, except for the main function’s name: “from bool … to what?” — Why not explicitly,
bitmap_from_bools
, or bools_to_bitmap
?– Konrad Rudolph
yesterday
yes @KonradRudolph, I couldn't think of a better name, except maybe
bools_to_unsigned
. Bitmap is a good keyword; edited.– Stack Danny
19 hours ago
yes @KonradRudolph, I couldn't think of a better name, except maybe
bools_to_unsigned
. Bitmap is a good keyword; edited.– Stack Danny
19 hours ago
add a comment |
up vote
8
down vote
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)) ...
Yes, but hard to understand. But thanks for suggestion.
– Andrew Truckle
yesterday
I compared compilers ability to simplify expression with your simplification as a reference: compiler explorer. gcc did not find your optimal version but its solution is still good. Clang and MSVC don't seem to perform any boolean expression simplification.
– Oliv
yesterday
1
@AndrewTruckle: note, that if you needed a more readable version, then please say so. You've said "simplified", yet you accept an even more verbose version than your original one.
– geza
yesterday
@Oliv: thanks for doing this test! This proves the point that while compilers are getting better and better, they still have a lot to learn :)
– geza
yesterday
1
simple
is indeed a vague term. Many people understand it in this context as simpler for developer to understand and not for the compiler to generate code, so more verbose can indeed be simpler.
– Zdeslav Vojkovic
yesterday
|
show 6 more comments
up vote
8
down vote
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)) ...
Yes, but hard to understand. But thanks for suggestion.
– Andrew Truckle
yesterday
I compared compilers ability to simplify expression with your simplification as a reference: compiler explorer. gcc did not find your optimal version but its solution is still good. Clang and MSVC don't seem to perform any boolean expression simplification.
– Oliv
yesterday
1
@AndrewTruckle: note, that if you needed a more readable version, then please say so. You've said "simplified", yet you accept an even more verbose version than your original one.
– geza
yesterday
@Oliv: thanks for doing this test! This proves the point that while compilers are getting better and better, they still have a lot to learn :)
– geza
yesterday
1
simple
is indeed a vague term. Many people understand it in this context as simpler for developer to understand and not for the compiler to generate code, so more verbose can indeed be simpler.
– Zdeslav Vojkovic
yesterday
|
show 6 more comments
up vote
8
down vote
up vote
8
down vote
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)) ...
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)) ...
edited yesterday
answered yesterday
geza
12.1k32772
12.1k32772
Yes, but hard to understand. But thanks for suggestion.
– Andrew Truckle
yesterday
I compared compilers ability to simplify expression with your simplification as a reference: compiler explorer. gcc did not find your optimal version but its solution is still good. Clang and MSVC don't seem to perform any boolean expression simplification.
– Oliv
yesterday
1
@AndrewTruckle: note, that if you needed a more readable version, then please say so. You've said "simplified", yet you accept an even more verbose version than your original one.
– geza
yesterday
@Oliv: thanks for doing this test! This proves the point that while compilers are getting better and better, they still have a lot to learn :)
– geza
yesterday
1
simple
is indeed a vague term. Many people understand it in this context as simpler for developer to understand and not for the compiler to generate code, so more verbose can indeed be simpler.
– Zdeslav Vojkovic
yesterday
|
show 6 more comments
Yes, but hard to understand. But thanks for suggestion.
– Andrew Truckle
yesterday
I compared compilers ability to simplify expression with your simplification as a reference: compiler explorer. gcc did not find your optimal version but its solution is still good. Clang and MSVC don't seem to perform any boolean expression simplification.
– Oliv
yesterday
1
@AndrewTruckle: note, that if you needed a more readable version, then please say so. You've said "simplified", yet you accept an even more verbose version than your original one.
– geza
yesterday
@Oliv: thanks for doing this test! This proves the point that while compilers are getting better and better, they still have a lot to learn :)
– geza
yesterday
1
simple
is indeed a vague term. Many people understand it in this context as simpler for developer to understand and not for the compiler to generate code, so more verbose can indeed be simpler.
– Zdeslav Vojkovic
yesterday
Yes, but hard to understand. But thanks for suggestion.
– Andrew Truckle
yesterday
Yes, but hard to understand. But thanks for suggestion.
– Andrew Truckle
yesterday
I compared compilers ability to simplify expression with your simplification as a reference: compiler explorer. gcc did not find your optimal version but its solution is still good. Clang and MSVC don't seem to perform any boolean expression simplification.
– Oliv
yesterday
I compared compilers ability to simplify expression with your simplification as a reference: compiler explorer. gcc did not find your optimal version but its solution is still good. Clang and MSVC don't seem to perform any boolean expression simplification.
– Oliv
yesterday
1
1
@AndrewTruckle: note, that if you needed a more readable version, then please say so. You've said "simplified", yet you accept an even more verbose version than your original one.
– geza
yesterday
@AndrewTruckle: note, that if you needed a more readable version, then please say so. You've said "simplified", yet you accept an even more verbose version than your original one.
– geza
yesterday
@Oliv: thanks for doing this test! This proves the point that while compilers are getting better and better, they still have a lot to learn :)
– geza
yesterday
@Oliv: thanks for doing this test! This proves the point that while compilers are getting better and better, they still have a lot to learn :)
– geza
yesterday
1
1
simple
is indeed a vague term. Many people understand it in this context as simpler for developer to understand and not for the compiler to generate code, so more verbose can indeed be simpler.– Zdeslav Vojkovic
yesterday
simple
is indeed a vague term. Many people understand it in this context as simpler for developer to understand and not for the compiler to generate code, so more verbose can indeed be simpler.– Zdeslav Vojkovic
yesterday
|
show 6 more comments
up vote
8
down vote
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 (scenari1 || 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 (scenari1or2 || scenario3) {
// Do whatever.
}
What's important here is not predicate logic or Karnaugh maps. It's describint 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.
1
+1 I am surprised there are 0 upvotes here. The solution is short, self-documenting (no comments needed), and easy to modify with little chance of introducing bugs. A clear favourite.
– RedFilter
11 hours ago
add a comment |
up vote
8
down vote
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 (scenari1 || 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 (scenari1or2 || scenario3) {
// Do whatever.
}
What's important here is not predicate logic or Karnaugh maps. It's describint 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.
1
+1 I am surprised there are 0 upvotes here. The solution is short, self-documenting (no comments needed), and easy to modify with little chance of introducing bugs. A clear favourite.
– RedFilter
11 hours ago
add a comment |
up vote
8
down vote
up vote
8
down vote
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 (scenari1 || 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 (scenari1or2 || scenario3) {
// Do whatever.
}
What's important here is not predicate logic or Karnaugh maps. It's describint 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.
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 (scenari1 || 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 (scenari1or2 || scenario3) {
// Do whatever.
}
What's important here is not predicate logic or Karnaugh maps. It's describint 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.
edited 8 hours ago
answered 13 hours ago
Anders
4,65652951
4,65652951
1
+1 I am surprised there are 0 upvotes here. The solution is short, self-documenting (no comments needed), and easy to modify with little chance of introducing bugs. A clear favourite.
– RedFilter
11 hours ago
add a comment |
1
+1 I am surprised there are 0 upvotes here. The solution is short, self-documenting (no comments needed), and easy to modify with little chance of introducing bugs. A clear favourite.
– RedFilter
11 hours ago
1
1
+1 I am surprised there are 0 upvotes here. The solution is short, self-documenting (no comments needed), and easy to modify with little chance of introducing bugs. A clear favourite.
– RedFilter
11 hours ago
+1 I am surprised there are 0 upvotes here. The solution is short, self-documenting (no comments needed), and easy to modify with little chance of introducing bugs. A clear favourite.
– RedFilter
11 hours ago
add a comment |
up vote
6
down vote
I'm not seeing any answers saying to name the scenarios, though the OP's solution does exactly that.
To me it is best to encapsulate the comment of what each scenario is into either a variable name or function name. You're more likely to ignore a comment than a name, and if your logic changes in the future you're more likely to change a name than a comment. You can't refactor a comment.
If you plan on reusing these scenarios outside of your function (or might want to), then make a function that says what it evaluates (constexpr
/noexcept
optional but recommended):
constexpr bool IsScenario1(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && b2 && b3 && b4; }
constexpr bool IsScenario2(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && b2 && b3 && !b4; }
constexpr bool IsScenario3(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && !b2 && !b3 && !b4; }
Make these class methods if possible (like in OP's solution). You can use variables inside of your function if you don't think you'll reuse the logic:
const auto is_scenario_1 = bValue1 && bValue2 && bValue3 && bValue4;
const auto is_scenario_2 = bvalue1 && bvalue2 && bValue3 && !bValue4;
const auto is_scenario_3 = bValue1 && !bValue2 && !bValue3 && !bValue4;
The compiler will most likely sort out that if bValue1 is false then all scenarios are false. Don't worry about making it fast, just correct and readable. If you profile your code and find this to be a bottleneck because the compiler generated sub-optimal code at -O2 or higher then try to rewrite it.
I like this slightly more than Gian Paolo's (already nice) solution: It avoids control flow and the use of a variable that is overwritten - more functional style.
– Dirk Herrmann
yesterday
add a comment |
up vote
6
down vote
I'm not seeing any answers saying to name the scenarios, though the OP's solution does exactly that.
To me it is best to encapsulate the comment of what each scenario is into either a variable name or function name. You're more likely to ignore a comment than a name, and if your logic changes in the future you're more likely to change a name than a comment. You can't refactor a comment.
If you plan on reusing these scenarios outside of your function (or might want to), then make a function that says what it evaluates (constexpr
/noexcept
optional but recommended):
constexpr bool IsScenario1(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && b2 && b3 && b4; }
constexpr bool IsScenario2(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && b2 && b3 && !b4; }
constexpr bool IsScenario3(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && !b2 && !b3 && !b4; }
Make these class methods if possible (like in OP's solution). You can use variables inside of your function if you don't think you'll reuse the logic:
const auto is_scenario_1 = bValue1 && bValue2 && bValue3 && bValue4;
const auto is_scenario_2 = bvalue1 && bvalue2 && bValue3 && !bValue4;
const auto is_scenario_3 = bValue1 && !bValue2 && !bValue3 && !bValue4;
The compiler will most likely sort out that if bValue1 is false then all scenarios are false. Don't worry about making it fast, just correct and readable. If you profile your code and find this to be a bottleneck because the compiler generated sub-optimal code at -O2 or higher then try to rewrite it.
I like this slightly more than Gian Paolo's (already nice) solution: It avoids control flow and the use of a variable that is overwritten - more functional style.
– Dirk Herrmann
yesterday
add a comment |
up vote
6
down vote
up vote
6
down vote
I'm not seeing any answers saying to name the scenarios, though the OP's solution does exactly that.
To me it is best to encapsulate the comment of what each scenario is into either a variable name or function name. You're more likely to ignore a comment than a name, and if your logic changes in the future you're more likely to change a name than a comment. You can't refactor a comment.
If you plan on reusing these scenarios outside of your function (or might want to), then make a function that says what it evaluates (constexpr
/noexcept
optional but recommended):
constexpr bool IsScenario1(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && b2 && b3 && b4; }
constexpr bool IsScenario2(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && b2 && b3 && !b4; }
constexpr bool IsScenario3(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && !b2 && !b3 && !b4; }
Make these class methods if possible (like in OP's solution). You can use variables inside of your function if you don't think you'll reuse the logic:
const auto is_scenario_1 = bValue1 && bValue2 && bValue3 && bValue4;
const auto is_scenario_2 = bvalue1 && bvalue2 && bValue3 && !bValue4;
const auto is_scenario_3 = bValue1 && !bValue2 && !bValue3 && !bValue4;
The compiler will most likely sort out that if bValue1 is false then all scenarios are false. Don't worry about making it fast, just correct and readable. If you profile your code and find this to be a bottleneck because the compiler generated sub-optimal code at -O2 or higher then try to rewrite it.
I'm not seeing any answers saying to name the scenarios, though the OP's solution does exactly that.
To me it is best to encapsulate the comment of what each scenario is into either a variable name or function name. You're more likely to ignore a comment than a name, and if your logic changes in the future you're more likely to change a name than a comment. You can't refactor a comment.
If you plan on reusing these scenarios outside of your function (or might want to), then make a function that says what it evaluates (constexpr
/noexcept
optional but recommended):
constexpr bool IsScenario1(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && b2 && b3 && b4; }
constexpr bool IsScenario2(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && b2 && b3 && !b4; }
constexpr bool IsScenario3(bool b1, bool b2, bool b3, bool b4) noexcept
{ return b1 && !b2 && !b3 && !b4; }
Make these class methods if possible (like in OP's solution). You can use variables inside of your function if you don't think you'll reuse the logic:
const auto is_scenario_1 = bValue1 && bValue2 && bValue3 && bValue4;
const auto is_scenario_2 = bvalue1 && bvalue2 && bValue3 && !bValue4;
const auto is_scenario_3 = bValue1 && !bValue2 && !bValue3 && !bValue4;
The compiler will most likely sort out that if bValue1 is false then all scenarios are false. Don't worry about making it fast, just correct and readable. If you profile your code and find this to be a bottleneck because the compiler generated sub-optimal code at -O2 or higher then try to rewrite it.
edited 21 hours ago
Andrew Truckle
5,04732145
5,04732145
answered yesterday
Erroneous
460410
460410
I like this slightly more than Gian Paolo's (already nice) solution: It avoids control flow and the use of a variable that is overwritten - more functional style.
– Dirk Herrmann
yesterday
add a comment |
I like this slightly more than Gian Paolo's (already nice) solution: It avoids control flow and the use of a variable that is overwritten - more functional style.
– Dirk Herrmann
yesterday
I like this slightly more than Gian Paolo's (already nice) solution: It avoids control flow and the use of a variable that is overwritten - more functional style.
– Dirk Herrmann
yesterday
I like this slightly more than Gian Paolo's (already nice) solution: It avoids control flow and the use of a variable that is overwritten - more functional style.
– Dirk Herrmann
yesterday
add a comment |
up vote
5
down vote
A C/C++ way
bool scenario[3][4] = {{true, true, true, true},
{true, true, true, false},
{true, false, false, false}};
bool CheckScenario(bool bValue1, bool bValue2, bool bValue3, bool bValue4)
{
bool temp = {bValue1, bValue2, bValue3, bValue4};
for(int i = 0 ; i < sizeof(scenario) / sizeof(scenario[0]); i++)
{
if(memcmp(temp, scenario[i], sizeof(temp)) == 0)
return true;
}
return false;
}
This approach is scalable as if the number of valid conditions grow, you easily just add more of them to scenario list.
Thank you for your answer.
– Andrew Truckle
yesterday
I'm pretty sure this is wrong, though. It assumes that the compiler uses only a single binary representation fortrue
. A compiler which uses "anything non-zero is true" causes this code to fail. Note thattrue
must convert to1
, it just doesn't need to be stored as such.
– MSalters
yesterday
@MSalters, tnx, I get your point and I am aware of that, kinda like2 is not equal to true but evaluates to true
, my code doesnt forceint 1 = true
and works as long as all true's are converted to same int value, SO here is my question: Why compiler should act random on converting true to underlying int, Can you please elaborate more?
– hessam hedieh
yesterday
Performing amemcmp
to test boolean conditions is not the C++ way, and I rather doubt that it’s an established C way, either.
– Konrad Rudolph
yesterday
@hessamhedieh: The problem in your logic is "converting true to underlying int". That is not how compilers work,
– MSalters
16 hours ago
add a comment |
up vote
5
down vote
A C/C++ way
bool scenario[3][4] = {{true, true, true, true},
{true, true, true, false},
{true, false, false, false}};
bool CheckScenario(bool bValue1, bool bValue2, bool bValue3, bool bValue4)
{
bool temp = {bValue1, bValue2, bValue3, bValue4};
for(int i = 0 ; i < sizeof(scenario) / sizeof(scenario[0]); i++)
{
if(memcmp(temp, scenario[i], sizeof(temp)) == 0)
return true;
}
return false;
}
This approach is scalable as if the number of valid conditions grow, you easily just add more of them to scenario list.
Thank you for your answer.
– Andrew Truckle
yesterday
I'm pretty sure this is wrong, though. It assumes that the compiler uses only a single binary representation fortrue
. A compiler which uses "anything non-zero is true" causes this code to fail. Note thattrue
must convert to1
, it just doesn't need to be stored as such.
– MSalters
yesterday
@MSalters, tnx, I get your point and I am aware of that, kinda like2 is not equal to true but evaluates to true
, my code doesnt forceint 1 = true
and works as long as all true's are converted to same int value, SO here is my question: Why compiler should act random on converting true to underlying int, Can you please elaborate more?
– hessam hedieh
yesterday
Performing amemcmp
to test boolean conditions is not the C++ way, and I rather doubt that it’s an established C way, either.
– Konrad Rudolph
yesterday
@hessamhedieh: The problem in your logic is "converting true to underlying int". That is not how compilers work,
– MSalters
16 hours ago
add a comment |
up vote
5
down vote
up vote
5
down vote
A C/C++ way
bool scenario[3][4] = {{true, true, true, true},
{true, true, true, false},
{true, false, false, false}};
bool CheckScenario(bool bValue1, bool bValue2, bool bValue3, bool bValue4)
{
bool temp = {bValue1, bValue2, bValue3, bValue4};
for(int i = 0 ; i < sizeof(scenario) / sizeof(scenario[0]); i++)
{
if(memcmp(temp, scenario[i], sizeof(temp)) == 0)
return true;
}
return false;
}
This approach is scalable as if the number of valid conditions grow, you easily just add more of them to scenario list.
A C/C++ way
bool scenario[3][4] = {{true, true, true, true},
{true, true, true, false},
{true, false, false, false}};
bool CheckScenario(bool bValue1, bool bValue2, bool bValue3, bool bValue4)
{
bool temp = {bValue1, bValue2, bValue3, bValue4};
for(int i = 0 ; i < sizeof(scenario) / sizeof(scenario[0]); i++)
{
if(memcmp(temp, scenario[i], sizeof(temp)) == 0)
return true;
}
return false;
}
This approach is scalable as if the number of valid conditions grow, you easily just add more of them to scenario list.
answered yesterday
hessam hedieh
1355
1355
Thank you for your answer.
– Andrew Truckle
yesterday
I'm pretty sure this is wrong, though. It assumes that the compiler uses only a single binary representation fortrue
. A compiler which uses "anything non-zero is true" causes this code to fail. Note thattrue
must convert to1
, it just doesn't need to be stored as such.
– MSalters
yesterday
@MSalters, tnx, I get your point and I am aware of that, kinda like2 is not equal to true but evaluates to true
, my code doesnt forceint 1 = true
and works as long as all true's are converted to same int value, SO here is my question: Why compiler should act random on converting true to underlying int, Can you please elaborate more?
– hessam hedieh
yesterday
Performing amemcmp
to test boolean conditions is not the C++ way, and I rather doubt that it’s an established C way, either.
– Konrad Rudolph
yesterday
@hessamhedieh: The problem in your logic is "converting true to underlying int". That is not how compilers work,
– MSalters
16 hours ago
add a comment |
Thank you for your answer.
– Andrew Truckle
yesterday
I'm pretty sure this is wrong, though. It assumes that the compiler uses only a single binary representation fortrue
. A compiler which uses "anything non-zero is true" causes this code to fail. Note thattrue
must convert to1
, it just doesn't need to be stored as such.
– MSalters
yesterday
@MSalters, tnx, I get your point and I am aware of that, kinda like2 is not equal to true but evaluates to true
, my code doesnt forceint 1 = true
and works as long as all true's are converted to same int value, SO here is my question: Why compiler should act random on converting true to underlying int, Can you please elaborate more?
– hessam hedieh
yesterday
Performing amemcmp
to test boolean conditions is not the C++ way, and I rather doubt that it’s an established C way, either.
– Konrad Rudolph
yesterday
@hessamhedieh: The problem in your logic is "converting true to underlying int". That is not how compilers work,
– MSalters
16 hours ago
Thank you for your answer.
– Andrew Truckle
yesterday
Thank you for your answer.
– Andrew Truckle
yesterday
I'm pretty sure this is wrong, though. It assumes that the compiler uses only a single binary representation for
true
. A compiler which uses "anything non-zero is true" causes this code to fail. Note that true
must convert to 1
, it just doesn't need to be stored as such.– MSalters
yesterday
I'm pretty sure this is wrong, though. It assumes that the compiler uses only a single binary representation for
true
. A compiler which uses "anything non-zero is true" causes this code to fail. Note that true
must convert to 1
, it just doesn't need to be stored as such.– MSalters
yesterday
@MSalters, tnx, I get your point and I am aware of that, kinda like
2 is not equal to true but evaluates to true
, my code doesnt force int 1 = true
and works as long as all true's are converted to same int value, SO here is my question: Why compiler should act random on converting true to underlying int, Can you please elaborate more?– hessam hedieh
yesterday
@MSalters, tnx, I get your point and I am aware of that, kinda like
2 is not equal to true but evaluates to true
, my code doesnt force int 1 = true
and works as long as all true's are converted to same int value, SO here is my question: Why compiler should act random on converting true to underlying int, Can you please elaborate more?– hessam hedieh
yesterday
Performing a
memcmp
to test boolean conditions is not the C++ way, and I rather doubt that it’s an established C way, either.– Konrad Rudolph
yesterday
Performing a
memcmp
to test boolean conditions is not the C++ way, and I rather doubt that it’s an established C way, either.– Konrad Rudolph
yesterday
@hessamhedieh: The problem in your logic is "converting true to underlying int". That is not how compilers work,
– MSalters
16 hours ago
@hessamhedieh: The problem in your logic is "converting true to underlying int". That is not how compilers work,
– MSalters
16 hours ago
add a comment |
up vote
5
down vote
I am only providing my answer here as in the comments someone suggested to show my solution. I want to thank everyone for their insights.
In the end I opted to add three new "scenario" boolean
methods:
bool CChristianLifeMinistryValidationDlg::IsFirstWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
return (INCLUDE_ITEM1(pEntry) &&
!INCLUDE_ITEM2(pEntry) &&
!INCLUDE_ITEM3(pEntry) &&
!INCLUDE_ITEM4(pEntry));
}
bool CChristianLifeMinistryValidationDlg::IsSecondWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
return (INCLUDE_ITEM1(pEntry) &&
INCLUDE_ITEM2(pEntry) &&
INCLUDE_ITEM3(pEntry) &&
INCLUDE_ITEM4(pEntry));
}
bool CChristianLifeMinistryValidationDlg::IsOtherWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
return (INCLUDE_ITEM1(pEntry) &&
INCLUDE_ITEM2(pEntry) &&
INCLUDE_ITEM3(pEntry) &&
!INCLUDE_ITEM4(pEntry));
}
Then I was able to apply those my my validation routine like this:
if (!IsFirstWeekStudentItems(pEntry) && !IsSecondWeekStudentItems(pEntry) && !IsOtherWeekStudentItems(pEntry))
{
; Error
}
In my live application the 4 bool values are actually extracted from a DWORD
which has 4 values encoded into it.
Thanks again everyone.
1
Thanks for sharing the solution. :) It's actually better than the complex if conditions hell. Maybe you can still nameINCLUDE_ITEM1
etc in a better way and you are all good. :)
– Hardik Modha
yesterday
1
@HardikModha Well, technically they are "Student items" and the flag is to indicate if they are to be "included". So I think the name, albeit sounding generic, is actually meaningful in this context. :)
– Andrew Truckle
yesterday
Well, Sounds good then. :)
– Hardik Modha
yesterday
add a comment |
up vote
5
down vote
I am only providing my answer here as in the comments someone suggested to show my solution. I want to thank everyone for their insights.
In the end I opted to add three new "scenario" boolean
methods:
bool CChristianLifeMinistryValidationDlg::IsFirstWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
return (INCLUDE_ITEM1(pEntry) &&
!INCLUDE_ITEM2(pEntry) &&
!INCLUDE_ITEM3(pEntry) &&
!INCLUDE_ITEM4(pEntry));
}
bool CChristianLifeMinistryValidationDlg::IsSecondWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
return (INCLUDE_ITEM1(pEntry) &&
INCLUDE_ITEM2(pEntry) &&
INCLUDE_ITEM3(pEntry) &&
INCLUDE_ITEM4(pEntry));
}
bool CChristianLifeMinistryValidationDlg::IsOtherWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
return (INCLUDE_ITEM1(pEntry) &&
INCLUDE_ITEM2(pEntry) &&
INCLUDE_ITEM3(pEntry) &&
!INCLUDE_ITEM4(pEntry));
}
Then I was able to apply those my my validation routine like this:
if (!IsFirstWeekStudentItems(pEntry) && !IsSecondWeekStudentItems(pEntry) && !IsOtherWeekStudentItems(pEntry))
{
; Error
}
In my live application the 4 bool values are actually extracted from a DWORD
which has 4 values encoded into it.
Thanks again everyone.
1
Thanks for sharing the solution. :) It's actually better than the complex if conditions hell. Maybe you can still nameINCLUDE_ITEM1
etc in a better way and you are all good. :)
– Hardik Modha
yesterday
1
@HardikModha Well, technically they are "Student items" and the flag is to indicate if they are to be "included". So I think the name, albeit sounding generic, is actually meaningful in this context. :)
– Andrew Truckle
yesterday
Well, Sounds good then. :)
– Hardik Modha
yesterday
add a comment |
up vote
5
down vote
up vote
5
down vote
I am only providing my answer here as in the comments someone suggested to show my solution. I want to thank everyone for their insights.
In the end I opted to add three new "scenario" boolean
methods:
bool CChristianLifeMinistryValidationDlg::IsFirstWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
return (INCLUDE_ITEM1(pEntry) &&
!INCLUDE_ITEM2(pEntry) &&
!INCLUDE_ITEM3(pEntry) &&
!INCLUDE_ITEM4(pEntry));
}
bool CChristianLifeMinistryValidationDlg::IsSecondWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
return (INCLUDE_ITEM1(pEntry) &&
INCLUDE_ITEM2(pEntry) &&
INCLUDE_ITEM3(pEntry) &&
INCLUDE_ITEM4(pEntry));
}
bool CChristianLifeMinistryValidationDlg::IsOtherWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
return (INCLUDE_ITEM1(pEntry) &&
INCLUDE_ITEM2(pEntry) &&
INCLUDE_ITEM3(pEntry) &&
!INCLUDE_ITEM4(pEntry));
}
Then I was able to apply those my my validation routine like this:
if (!IsFirstWeekStudentItems(pEntry) && !IsSecondWeekStudentItems(pEntry) && !IsOtherWeekStudentItems(pEntry))
{
; Error
}
In my live application the 4 bool values are actually extracted from a DWORD
which has 4 values encoded into it.
Thanks again everyone.
I am only providing my answer here as in the comments someone suggested to show my solution. I want to thank everyone for their insights.
In the end I opted to add three new "scenario" boolean
methods:
bool CChristianLifeMinistryValidationDlg::IsFirstWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
return (INCLUDE_ITEM1(pEntry) &&
!INCLUDE_ITEM2(pEntry) &&
!INCLUDE_ITEM3(pEntry) &&
!INCLUDE_ITEM4(pEntry));
}
bool CChristianLifeMinistryValidationDlg::IsSecondWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
return (INCLUDE_ITEM1(pEntry) &&
INCLUDE_ITEM2(pEntry) &&
INCLUDE_ITEM3(pEntry) &&
INCLUDE_ITEM4(pEntry));
}
bool CChristianLifeMinistryValidationDlg::IsOtherWeekStudentItems(CChristianLifeMinistryEntry *pEntry)
{
return (INCLUDE_ITEM1(pEntry) &&
INCLUDE_ITEM2(pEntry) &&
INCLUDE_ITEM3(pEntry) &&
!INCLUDE_ITEM4(pEntry));
}
Then I was able to apply those my my validation routine like this:
if (!IsFirstWeekStudentItems(pEntry) && !IsSecondWeekStudentItems(pEntry) && !IsOtherWeekStudentItems(pEntry))
{
; Error
}
In my live application the 4 bool values are actually extracted from a DWORD
which has 4 values encoded into it.
Thanks again everyone.
answered yesterday
Andrew Truckle
5,04732145
5,04732145
1
Thanks for sharing the solution. :) It's actually better than the complex if conditions hell. Maybe you can still nameINCLUDE_ITEM1
etc in a better way and you are all good. :)
– Hardik Modha
yesterday
1
@HardikModha Well, technically they are "Student items" and the flag is to indicate if they are to be "included". So I think the name, albeit sounding generic, is actually meaningful in this context. :)
– Andrew Truckle
yesterday
Well, Sounds good then. :)
– Hardik Modha
yesterday
add a comment |
1
Thanks for sharing the solution. :) It's actually better than the complex if conditions hell. Maybe you can still nameINCLUDE_ITEM1
etc in a better way and you are all good. :)
– Hardik Modha
yesterday
1
@HardikModha Well, technically they are "Student items" and the flag is to indicate if they are to be "included". So I think the name, albeit sounding generic, is actually meaningful in this context. :)
– Andrew Truckle
yesterday
Well, Sounds good then. :)
– Hardik Modha
yesterday
1
1
Thanks for sharing the solution. :) It's actually better than the complex if conditions hell. Maybe you can still name
INCLUDE_ITEM1
etc in a better way and you are all good. :)– Hardik Modha
yesterday
Thanks for sharing the solution. :) It's actually better than the complex if conditions hell. Maybe you can still name
INCLUDE_ITEM1
etc in a better way and you are all good. :)– Hardik Modha
yesterday
1
1
@HardikModha Well, technically they are "Student items" and the flag is to indicate if they are to be "included". So I think the name, albeit sounding generic, is actually meaningful in this context. :)
– Andrew Truckle
yesterday
@HardikModha Well, technically they are "Student items" and the flag is to indicate if they are to be "included". So I think the name, albeit sounding generic, is actually meaningful in this context. :)
– Andrew Truckle
yesterday
Well, Sounds good then. :)
– Hardik Modha
yesterday
Well, Sounds good then. :)
– Hardik Modha
yesterday
add a comment |
up vote
4
down vote
It's easy to notice that first two scenarios are similar - they share most of the conditions. If you want to select in which scenario you are at the moment, you could write it like this (it's a modified @gian-paolo's solution):
bool valid = false;
if(bValue1 && bValue2 && bValue3)
{
if (bValue4)
valid = true; //scenario 1
else if (!bValue4)
valid = true; //scenario 2
}
else if (bValue1 && !bValue2 && !bValue3 && !bValue4)
valid = true; //scenario 3
Going further, you can notice, that first boolean needs to be always true, which is an entry condition, so you can end up with:
bool valid = false;
if(bValue1)
{
if(bValue2 && bValue3)
{
if (bValue4)
valid = true; //scenario 1
else if (!bValue4)
valid = true; //scenario 2
}
else if (!bValue2 && !bValue3 && !bValue4)
valid = true; //scenario 3
}
Even more, you can now clearly see, that bValue2 and bValue3 are somewhat connected - you could extract their state to some external functions or variables with more appropriate name (this is not always easy or appropriate though):
bool valid = false;
if(bValue1)
{
bool bValue1and2 = bValue1 && bValue2;
bool notBValue1and2 = !bValue2 && !bValue3;
if(bValue1and2)
{
if (bValue4)
valid = true; //scenario 1
else if (!bValue4)
valid = true; //scenario 2
}
else if (notBValue1and2 && !bValue4)
valid = true; //scenario 3
}
Doing it this way have some advantages and disadvantages:
- conditions are smaller, so it's easier to reason about them,
- it's easier to do nice renaming to make these conditions more understandable,
- but, they require to understand the scope,
- moreover it's more rigid
If you predict that there will be changes to the above logic, you should use more straightforward approach as presented by @gian-paolo.
Otherwise, if these conditions are well established, and are kind of "solid rules" that will never change, consider my last code snippet.
add a comment |
up vote
4
down vote
It's easy to notice that first two scenarios are similar - they share most of the conditions. If you want to select in which scenario you are at the moment, you could write it like this (it's a modified @gian-paolo's solution):
bool valid = false;
if(bValue1 && bValue2 && bValue3)
{
if (bValue4)
valid = true; //scenario 1
else if (!bValue4)
valid = true; //scenario 2
}
else if (bValue1 && !bValue2 && !bValue3 && !bValue4)
valid = true; //scenario 3
Going further, you can notice, that first boolean needs to be always true, which is an entry condition, so you can end up with:
bool valid = false;
if(bValue1)
{
if(bValue2 && bValue3)
{
if (bValue4)
valid = true; //scenario 1
else if (!bValue4)
valid = true; //scenario 2
}
else if (!bValue2 && !bValue3 && !bValue4)
valid = true; //scenario 3
}
Even more, you can now clearly see, that bValue2 and bValue3 are somewhat connected - you could extract their state to some external functions or variables with more appropriate name (this is not always easy or appropriate though):
bool valid = false;
if(bValue1)
{
bool bValue1and2 = bValue1 && bValue2;
bool notBValue1and2 = !bValue2 && !bValue3;
if(bValue1and2)
{
if (bValue4)
valid = true; //scenario 1
else if (!bValue4)
valid = true; //scenario 2
}
else if (notBValue1and2 && !bValue4)
valid = true; //scenario 3
}
Doing it this way have some advantages and disadvantages:
- conditions are smaller, so it's easier to reason about them,
- it's easier to do nice renaming to make these conditions more understandable,
- but, they require to understand the scope,
- moreover it's more rigid
If you predict that there will be changes to the above logic, you should use more straightforward approach as presented by @gian-paolo.
Otherwise, if these conditions are well established, and are kind of "solid rules" that will never change, consider my last code snippet.
add a comment |
up vote
4
down vote
up vote
4
down vote
It's easy to notice that first two scenarios are similar - they share most of the conditions. If you want to select in which scenario you are at the moment, you could write it like this (it's a modified @gian-paolo's solution):
bool valid = false;
if(bValue1 && bValue2 && bValue3)
{
if (bValue4)
valid = true; //scenario 1
else if (!bValue4)
valid = true; //scenario 2
}
else if (bValue1 && !bValue2 && !bValue3 && !bValue4)
valid = true; //scenario 3
Going further, you can notice, that first boolean needs to be always true, which is an entry condition, so you can end up with:
bool valid = false;
if(bValue1)
{
if(bValue2 && bValue3)
{
if (bValue4)
valid = true; //scenario 1
else if (!bValue4)
valid = true; //scenario 2
}
else if (!bValue2 && !bValue3 && !bValue4)
valid = true; //scenario 3
}
Even more, you can now clearly see, that bValue2 and bValue3 are somewhat connected - you could extract their state to some external functions or variables with more appropriate name (this is not always easy or appropriate though):
bool valid = false;
if(bValue1)
{
bool bValue1and2 = bValue1 && bValue2;
bool notBValue1and2 = !bValue2 && !bValue3;
if(bValue1and2)
{
if (bValue4)
valid = true; //scenario 1
else if (!bValue4)
valid = true; //scenario 2
}
else if (notBValue1and2 && !bValue4)
valid = true; //scenario 3
}
Doing it this way have some advantages and disadvantages:
- conditions are smaller, so it's easier to reason about them,
- it's easier to do nice renaming to make these conditions more understandable,
- but, they require to understand the scope,
- moreover it's more rigid
If you predict that there will be changes to the above logic, you should use more straightforward approach as presented by @gian-paolo.
Otherwise, if these conditions are well established, and are kind of "solid rules" that will never change, consider my last code snippet.
It's easy to notice that first two scenarios are similar - they share most of the conditions. If you want to select in which scenario you are at the moment, you could write it like this (it's a modified @gian-paolo's solution):
bool valid = false;
if(bValue1 && bValue2 && bValue3)
{
if (bValue4)
valid = true; //scenario 1
else if (!bValue4)
valid = true; //scenario 2
}
else if (bValue1 && !bValue2 && !bValue3 && !bValue4)
valid = true; //scenario 3
Going further, you can notice, that first boolean needs to be always true, which is an entry condition, so you can end up with:
bool valid = false;
if(bValue1)
{
if(bValue2 && bValue3)
{
if (bValue4)
valid = true; //scenario 1
else if (!bValue4)
valid = true; //scenario 2
}
else if (!bValue2 && !bValue3 && !bValue4)
valid = true; //scenario 3
}
Even more, you can now clearly see, that bValue2 and bValue3 are somewhat connected - you could extract their state to some external functions or variables with more appropriate name (this is not always easy or appropriate though):
bool valid = false;
if(bValue1)
{
bool bValue1and2 = bValue1 && bValue2;
bool notBValue1and2 = !bValue2 && !bValue3;
if(bValue1and2)
{
if (bValue4)
valid = true; //scenario 1
else if (!bValue4)
valid = true; //scenario 2
}
else if (notBValue1and2 && !bValue4)
valid = true; //scenario 3
}
Doing it this way have some advantages and disadvantages:
- conditions are smaller, so it's easier to reason about them,
- it's easier to do nice renaming to make these conditions more understandable,
- but, they require to understand the scope,
- moreover it's more rigid
If you predict that there will be changes to the above logic, you should use more straightforward approach as presented by @gian-paolo.
Otherwise, if these conditions are well established, and are kind of "solid rules" that will never change, consider my last code snippet.
answered yesterday
Michał Łoś
336210
336210
add a comment |
add a comment |
up vote
3
down vote
I would also use shortcut variables for clarity. As noted earlier scenario 1 equals to scenario 2, because the value of bValue4 doesn't influence the truth of those two scenarios.
bool MAJORLY_TRUE=bValue1 && bValue2 && bValue3
bool MAJORLY_FALSE=!(bValue2 || bValue3 || bValue4)
then your expression beomes:
if (MAJORLY_TRUE || (bValue1 && MAJORLY_FALSE))
{
// do something
}
else
{
// There is some error
}
Giving meaningful names to MAJORTRUE and MAJORFALSE variables (as well as actually to bValue* vars) would help a lot with readability and maintenance.
add a comment |
up vote
3
down vote
I would also use shortcut variables for clarity. As noted earlier scenario 1 equals to scenario 2, because the value of bValue4 doesn't influence the truth of those two scenarios.
bool MAJORLY_TRUE=bValue1 && bValue2 && bValue3
bool MAJORLY_FALSE=!(bValue2 || bValue3 || bValue4)
then your expression beomes:
if (MAJORLY_TRUE || (bValue1 && MAJORLY_FALSE))
{
// do something
}
else
{
// There is some error
}
Giving meaningful names to MAJORTRUE and MAJORFALSE variables (as well as actually to bValue* vars) would help a lot with readability and maintenance.
add a comment |
up vote
3
down vote
up vote
3
down vote
I would also use shortcut variables for clarity. As noted earlier scenario 1 equals to scenario 2, because the value of bValue4 doesn't influence the truth of those two scenarios.
bool MAJORLY_TRUE=bValue1 && bValue2 && bValue3
bool MAJORLY_FALSE=!(bValue2 || bValue3 || bValue4)
then your expression beomes:
if (MAJORLY_TRUE || (bValue1 && MAJORLY_FALSE))
{
// do something
}
else
{
// There is some error
}
Giving meaningful names to MAJORTRUE and MAJORFALSE variables (as well as actually to bValue* vars) would help a lot with readability and maintenance.
I would also use shortcut variables for clarity. As noted earlier scenario 1 equals to scenario 2, because the value of bValue4 doesn't influence the truth of those two scenarios.
bool MAJORLY_TRUE=bValue1 && bValue2 && bValue3
bool MAJORLY_FALSE=!(bValue2 || bValue3 || bValue4)
then your expression beomes:
if (MAJORLY_TRUE || (bValue1 && MAJORLY_FALSE))
{
// do something
}
else
{
// There is some error
}
Giving meaningful names to MAJORTRUE and MAJORFALSE variables (as well as actually to bValue* vars) would help a lot with readability and maintenance.
answered yesterday
Gnudiff
3,24111721
3,24111721
add a comment |
add a comment |
up vote
3
down vote
As suggested by mch, you could do:
if(!((bValue1 && bValue2 && bValue3) ||
(bValue1 && !bValue2 && !bValue3 && !bValue4))
)
where the first line covers the two first good cases, and the second line covers the last one.
Live Demo, where I played around and it passes your cases.
add a comment |
up vote
3
down vote
As suggested by mch, you could do:
if(!((bValue1 && bValue2 && bValue3) ||
(bValue1 && !bValue2 && !bValue3 && !bValue4))
)
where the first line covers the two first good cases, and the second line covers the last one.
Live Demo, where I played around and it passes your cases.
add a comment |
up vote
3
down vote
up vote
3
down vote
As suggested by mch, you could do:
if(!((bValue1 && bValue2 && bValue3) ||
(bValue1 && !bValue2 && !bValue3 && !bValue4))
)
where the first line covers the two first good cases, and the second line covers the last one.
Live Demo, where I played around and it passes your cases.
As suggested by mch, you could do:
if(!((bValue1 && bValue2 && bValue3) ||
(bValue1 && !bValue2 && !bValue3 && !bValue4))
)
where the first line covers the two first good cases, and the second line covers the last one.
Live Demo, where I played around and it passes your cases.
edited yesterday
answered yesterday
gsamaras
48.9k2397179
48.9k2397179
add a comment |
add a comment |
up vote
3
down vote
Focus on readability of the problem, not the specific "if" statement.
While this will produce more lines of code, and some may consider it either overkill or unnecessary. I'd suggest that abstracting your scenarios from the specific booleans is the best way to maintain readability.
By splitting things into classes (feel free to just use functions, or whatever other tool you prefer) with understandable names - we can much more easily show the meanings behind each scenario. More importantly, in a system with many moving parts - it is easier to maintain and join into your existing systems (again, despite how much extra code is involed).
#include <iostream>
#include <vector>
using namespace std;
// These values would likely not come from a single struct in real life
// Instead, they may be references to other booleans in other systems
struct Values
{
bool bValue1; // These would be given better names in reality
bool bValue2; // e.g. bDidTheCarCatchFire
bool bValue3; // and bDidTheWindshieldFallOff
bool bValue4;
};
class Scenario
{
public:
Scenario(Values& values)
: mValues(values) {}
virtual operator bool() = 0;
protected:
Values& mValues;
};
// Names as examples of things that describe your "scenarios" more effectively
class Scenario1_TheCarWasNotDamagedAtAll : public Scenario
{
public:
Scenario1_TheCarWasNotDamagedAtAll(Values& values) : Scenario(values) {}
virtual operator bool()
{
return mValues.bValue1
&& mValues.bValue2
&& mValues.bValue3
&& mValues.bValue4;
}
};
class Scenario2_TheCarBreaksDownButDidntGoOnFire : public Scenario
{
public:
Scenario2_TheCarBreaksDownButDidntGoOnFire(Values& values) : Scenario(values) {}
virtual operator bool()
{
return mValues.bValue1
&& mValues.bValue2
&& mValues.bValue3
&& !mValues.bValue4;
}
};
class Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere : public Scenario
{
public:
Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere(Values& values) : Scenario(values) {}
virtual operator bool()
{
return mValues.bValue1
&& !mValues.bValue2
&& !mValues.bValue3
&& !mValues.bValue4;
}
};
Scenario* findMatchingScenario(std::vector<Scenario*>& scenarios)
{
for(std::vector<Scenario*>::iterator it = scenarios.begin(); it != scenarios.end(); it++)
{
if (**it)
{
return *it;
}
}
return NULL;
}
int main() {
Values values = {true, true, true, true};
std::vector<Scenario*> scenarios = {
new Scenario1_TheCarWasNotDamagedAtAll(values),
new Scenario2_TheCarBreaksDownButDidntGoOnFire(values),
new Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere(values)
};
Scenario* matchingScenario = findMatchingScenario(scenarios);
if(matchingScenario)
{
std::cout << matchingScenario << " was a match" << std::endl;
}
else
{
std::cout << "No match" << std::endl;
}
// your code goes here
return 0;
}
5
At some point, verbosity starts to harm readability. I think this goes too far.
– JollyJoker
yesterday
1
@JollyJoker I do actually agree in this specific situation - however, my gut feeling from the way OP has named everything extremely generically, is that their "real" code is likely a lot more complex than the example they've given. Really, I just wanted to put this alternative out there, as it's how I'd structure it for something far more complex/involved. But you're right - for OPs specific example, it is overly verbose and makes matters worse.
– Bilkokuya
yesterday
add a comment |
up vote
3
down vote
Focus on readability of the problem, not the specific "if" statement.
While this will produce more lines of code, and some may consider it either overkill or unnecessary. I'd suggest that abstracting your scenarios from the specific booleans is the best way to maintain readability.
By splitting things into classes (feel free to just use functions, or whatever other tool you prefer) with understandable names - we can much more easily show the meanings behind each scenario. More importantly, in a system with many moving parts - it is easier to maintain and join into your existing systems (again, despite how much extra code is involed).
#include <iostream>
#include <vector>
using namespace std;
// These values would likely not come from a single struct in real life
// Instead, they may be references to other booleans in other systems
struct Values
{
bool bValue1; // These would be given better names in reality
bool bValue2; // e.g. bDidTheCarCatchFire
bool bValue3; // and bDidTheWindshieldFallOff
bool bValue4;
};
class Scenario
{
public:
Scenario(Values& values)
: mValues(values) {}
virtual operator bool() = 0;
protected:
Values& mValues;
};
// Names as examples of things that describe your "scenarios" more effectively
class Scenario1_TheCarWasNotDamagedAtAll : public Scenario
{
public:
Scenario1_TheCarWasNotDamagedAtAll(Values& values) : Scenario(values) {}
virtual operator bool()
{
return mValues.bValue1
&& mValues.bValue2
&& mValues.bValue3
&& mValues.bValue4;
}
};
class Scenario2_TheCarBreaksDownButDidntGoOnFire : public Scenario
{
public:
Scenario2_TheCarBreaksDownButDidntGoOnFire(Values& values) : Scenario(values) {}
virtual operator bool()
{
return mValues.bValue1
&& mValues.bValue2
&& mValues.bValue3
&& !mValues.bValue4;
}
};
class Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere : public Scenario
{
public:
Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere(Values& values) : Scenario(values) {}
virtual operator bool()
{
return mValues.bValue1
&& !mValues.bValue2
&& !mValues.bValue3
&& !mValues.bValue4;
}
};
Scenario* findMatchingScenario(std::vector<Scenario*>& scenarios)
{
for(std::vector<Scenario*>::iterator it = scenarios.begin(); it != scenarios.end(); it++)
{
if (**it)
{
return *it;
}
}
return NULL;
}
int main() {
Values values = {true, true, true, true};
std::vector<Scenario*> scenarios = {
new Scenario1_TheCarWasNotDamagedAtAll(values),
new Scenario2_TheCarBreaksDownButDidntGoOnFire(values),
new Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere(values)
};
Scenario* matchingScenario = findMatchingScenario(scenarios);
if(matchingScenario)
{
std::cout << matchingScenario << " was a match" << std::endl;
}
else
{
std::cout << "No match" << std::endl;
}
// your code goes here
return 0;
}
5
At some point, verbosity starts to harm readability. I think this goes too far.
– JollyJoker
yesterday
1
@JollyJoker I do actually agree in this specific situation - however, my gut feeling from the way OP has named everything extremely generically, is that their "real" code is likely a lot more complex than the example they've given. Really, I just wanted to put this alternative out there, as it's how I'd structure it for something far more complex/involved. But you're right - for OPs specific example, it is overly verbose and makes matters worse.
– Bilkokuya
yesterday
add a comment |
up vote
3
down vote
up vote
3
down vote
Focus on readability of the problem, not the specific "if" statement.
While this will produce more lines of code, and some may consider it either overkill or unnecessary. I'd suggest that abstracting your scenarios from the specific booleans is the best way to maintain readability.
By splitting things into classes (feel free to just use functions, or whatever other tool you prefer) with understandable names - we can much more easily show the meanings behind each scenario. More importantly, in a system with many moving parts - it is easier to maintain and join into your existing systems (again, despite how much extra code is involed).
#include <iostream>
#include <vector>
using namespace std;
// These values would likely not come from a single struct in real life
// Instead, they may be references to other booleans in other systems
struct Values
{
bool bValue1; // These would be given better names in reality
bool bValue2; // e.g. bDidTheCarCatchFire
bool bValue3; // and bDidTheWindshieldFallOff
bool bValue4;
};
class Scenario
{
public:
Scenario(Values& values)
: mValues(values) {}
virtual operator bool() = 0;
protected:
Values& mValues;
};
// Names as examples of things that describe your "scenarios" more effectively
class Scenario1_TheCarWasNotDamagedAtAll : public Scenario
{
public:
Scenario1_TheCarWasNotDamagedAtAll(Values& values) : Scenario(values) {}
virtual operator bool()
{
return mValues.bValue1
&& mValues.bValue2
&& mValues.bValue3
&& mValues.bValue4;
}
};
class Scenario2_TheCarBreaksDownButDidntGoOnFire : public Scenario
{
public:
Scenario2_TheCarBreaksDownButDidntGoOnFire(Values& values) : Scenario(values) {}
virtual operator bool()
{
return mValues.bValue1
&& mValues.bValue2
&& mValues.bValue3
&& !mValues.bValue4;
}
};
class Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere : public Scenario
{
public:
Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere(Values& values) : Scenario(values) {}
virtual operator bool()
{
return mValues.bValue1
&& !mValues.bValue2
&& !mValues.bValue3
&& !mValues.bValue4;
}
};
Scenario* findMatchingScenario(std::vector<Scenario*>& scenarios)
{
for(std::vector<Scenario*>::iterator it = scenarios.begin(); it != scenarios.end(); it++)
{
if (**it)
{
return *it;
}
}
return NULL;
}
int main() {
Values values = {true, true, true, true};
std::vector<Scenario*> scenarios = {
new Scenario1_TheCarWasNotDamagedAtAll(values),
new Scenario2_TheCarBreaksDownButDidntGoOnFire(values),
new Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere(values)
};
Scenario* matchingScenario = findMatchingScenario(scenarios);
if(matchingScenario)
{
std::cout << matchingScenario << " was a match" << std::endl;
}
else
{
std::cout << "No match" << std::endl;
}
// your code goes here
return 0;
}
Focus on readability of the problem, not the specific "if" statement.
While this will produce more lines of code, and some may consider it either overkill or unnecessary. I'd suggest that abstracting your scenarios from the specific booleans is the best way to maintain readability.
By splitting things into classes (feel free to just use functions, or whatever other tool you prefer) with understandable names - we can much more easily show the meanings behind each scenario. More importantly, in a system with many moving parts - it is easier to maintain and join into your existing systems (again, despite how much extra code is involed).
#include <iostream>
#include <vector>
using namespace std;
// These values would likely not come from a single struct in real life
// Instead, they may be references to other booleans in other systems
struct Values
{
bool bValue1; // These would be given better names in reality
bool bValue2; // e.g. bDidTheCarCatchFire
bool bValue3; // and bDidTheWindshieldFallOff
bool bValue4;
};
class Scenario
{
public:
Scenario(Values& values)
: mValues(values) {}
virtual operator bool() = 0;
protected:
Values& mValues;
};
// Names as examples of things that describe your "scenarios" more effectively
class Scenario1_TheCarWasNotDamagedAtAll : public Scenario
{
public:
Scenario1_TheCarWasNotDamagedAtAll(Values& values) : Scenario(values) {}
virtual operator bool()
{
return mValues.bValue1
&& mValues.bValue2
&& mValues.bValue3
&& mValues.bValue4;
}
};
class Scenario2_TheCarBreaksDownButDidntGoOnFire : public Scenario
{
public:
Scenario2_TheCarBreaksDownButDidntGoOnFire(Values& values) : Scenario(values) {}
virtual operator bool()
{
return mValues.bValue1
&& mValues.bValue2
&& mValues.bValue3
&& !mValues.bValue4;
}
};
class Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere : public Scenario
{
public:
Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere(Values& values) : Scenario(values) {}
virtual operator bool()
{
return mValues.bValue1
&& !mValues.bValue2
&& !mValues.bValue3
&& !mValues.bValue4;
}
};
Scenario* findMatchingScenario(std::vector<Scenario*>& scenarios)
{
for(std::vector<Scenario*>::iterator it = scenarios.begin(); it != scenarios.end(); it++)
{
if (**it)
{
return *it;
}
}
return NULL;
}
int main() {
Values values = {true, true, true, true};
std::vector<Scenario*> scenarios = {
new Scenario1_TheCarWasNotDamagedAtAll(values),
new Scenario2_TheCarBreaksDownButDidntGoOnFire(values),
new Scenario3_TheCarWasCompletelyWreckedAndFireEverywhere(values)
};
Scenario* matchingScenario = findMatchingScenario(scenarios);
if(matchingScenario)
{
std::cout << matchingScenario << " was a match" << std::endl;
}
else
{
std::cout << "No match" << std::endl;
}
// your code goes here
return 0;
}
answered yesterday
Bilkokuya
750616
750616
5
At some point, verbosity starts to harm readability. I think this goes too far.
– JollyJoker
yesterday
1
@JollyJoker I do actually agree in this specific situation - however, my gut feeling from the way OP has named everything extremely generically, is that their "real" code is likely a lot more complex than the example they've given. Really, I just wanted to put this alternative out there, as it's how I'd structure it for something far more complex/involved. But you're right - for OPs specific example, it is overly verbose and makes matters worse.
– Bilkokuya
yesterday
add a comment |
5
At some point, verbosity starts to harm readability. I think this goes too far.
– JollyJoker
yesterday
1
@JollyJoker I do actually agree in this specific situation - however, my gut feeling from the way OP has named everything extremely generically, is that their "real" code is likely a lot more complex than the example they've given. Really, I just wanted to put this alternative out there, as it's how I'd structure it for something far more complex/involved. But you're right - for OPs specific example, it is overly verbose and makes matters worse.
– Bilkokuya
yesterday
5
5
At some point, verbosity starts to harm readability. I think this goes too far.
– JollyJoker
yesterday
At some point, verbosity starts to harm readability. I think this goes too far.
– JollyJoker
yesterday
1
1
@JollyJoker I do actually agree in this specific situation - however, my gut feeling from the way OP has named everything extremely generically, is that their "real" code is likely a lot more complex than the example they've given. Really, I just wanted to put this alternative out there, as it's how I'd structure it for something far more complex/involved. But you're right - for OPs specific example, it is overly verbose and makes matters worse.
– Bilkokuya
yesterday
@JollyJoker I do actually agree in this specific situation - however, my gut feeling from the way OP has named everything extremely generically, is that their "real" code is likely a lot more complex than the example they've given. Really, I just wanted to put this alternative out there, as it's how I'd structure it for something far more complex/involved. But you're right - for OPs specific example, it is overly verbose and makes matters worse.
– Bilkokuya
yesterday
add a comment |
up vote
2
down vote
I am denoting a, b, c, d for clarity, and A, B, C, D for complements
bValue1 = a (!A)
bValue2 = b (!B)
bValue3 = c (!C)
bValue4 = d (!D)
Equation
1 = abcd + abcD + aBCD
= a (bcd + bcD + BCD)
= a (bc + BCD)
= a (bcd + D (b ^C))
Use any equations that suits you.
add a comment |
up vote
2
down vote
I am denoting a, b, c, d for clarity, and A, B, C, D for complements
bValue1 = a (!A)
bValue2 = b (!B)
bValue3 = c (!C)
bValue4 = d (!D)
Equation
1 = abcd + abcD + aBCD
= a (bcd + bcD + BCD)
= a (bc + BCD)
= a (bcd + D (b ^C))
Use any equations that suits you.
add a comment |
up vote
2
down vote
up vote
2
down vote
I am denoting a, b, c, d for clarity, and A, B, C, D for complements
bValue1 = a (!A)
bValue2 = b (!B)
bValue3 = c (!C)
bValue4 = d (!D)
Equation
1 = abcd + abcD + aBCD
= a (bcd + bcD + BCD)
= a (bc + BCD)
= a (bcd + D (b ^C))
Use any equations that suits you.
I am denoting a, b, c, d for clarity, and A, B, C, D for complements
bValue1 = a (!A)
bValue2 = b (!B)
bValue3 = c (!C)
bValue4 = d (!D)
Equation
1 = abcd + abcD + aBCD
= a (bcd + bcD + BCD)
= a (bc + BCD)
= a (bcd + D (b ^C))
Use any equations that suits you.
answered yesterday
yumoji
1,40211123
1,40211123
add a comment |
add a comment |
up vote
2
down vote
A slight variation on @GianPaolo's fine answer, which some may find easier to read:
bool any_of_three_scenarios(bool v1, bool v2, bool v3, bool v4)
{
return (v1 && v2 && v3 && v4) // scenario 1
|| (v1 && v2 && v3 && !v4) // scenario 2
|| (v1 && !v2 && !v3 && !v4); // scenario 3
}
if (any_of_three_scenarios(bValue1,bValue2,bValue3,bValue4))
{
// ...
}
add a comment |
up vote
2
down vote
A slight variation on @GianPaolo's fine answer, which some may find easier to read:
bool any_of_three_scenarios(bool v1, bool v2, bool v3, bool v4)
{
return (v1 && v2 && v3 && v4) // scenario 1
|| (v1 && v2 && v3 && !v4) // scenario 2
|| (v1 && !v2 && !v3 && !v4); // scenario 3
}
if (any_of_three_scenarios(bValue1,bValue2,bValue3,bValue4))
{
// ...
}
add a comment |
up vote
2
down vote
up vote
2
down vote
A slight variation on @GianPaolo's fine answer, which some may find easier to read:
bool any_of_three_scenarios(bool v1, bool v2, bool v3, bool v4)
{
return (v1 && v2 && v3 && v4) // scenario 1
|| (v1 && v2 && v3 && !v4) // scenario 2
|| (v1 && !v2 && !v3 && !v4); // scenario 3
}
if (any_of_three_scenarios(bValue1,bValue2,bValue3,bValue4))
{
// ...
}
A slight variation on @GianPaolo's fine answer, which some may find easier to read:
bool any_of_three_scenarios(bool v1, bool v2, bool v3, bool v4)
{
return (v1 && v2 && v3 && v4) // scenario 1
|| (v1 && v2 && v3 && !v4) // scenario 2
|| (v1 && !v2 && !v3 && !v4); // scenario 3
}
if (any_of_three_scenarios(bValue1,bValue2,bValue3,bValue4))
{
// ...
}
answered yesterday
Matt
15.1k13447
15.1k13447
add a comment |
add a comment |
up vote
2
down vote
Consider translating your tables as directly as possible into your program. Drive the program based off the table, instead of mimicing it with logic.
template<class T0>
auto is_any_of( T0 const& t0, std::initializer_list<T0> il ) {
for (auto&& x:il)
if (x==t0) return true;
return false;
}
now
if (is_any_of(
std::make_tuple(bValue1, bValue2, bValue3, bValue4),
{
{true, true, true, true},
{true, true, true, false},
{true, false, false, false}
}
))
this directly as possible encodes your truth table into the compiler.
Live example.
You could also use std::any_of
directly:
using entry = std::array<bool, 4>;
constexpr entry acceptable =
{
{true, true, true, true},
{true, true, true, false},
{true, false, false, false}
};
if (std::any_of( begin(acceptable), end(acceptable), [&](auto&&x){
return entry{bValue1, bValue2, bValue3, bValue4} == x;
}) {
}
the compiler can inline the code, and eliminate any iteration and build its own logic for you. Meanwhile, your code reflects exactly how you concieved of the problem.
The first version is so easy to read and so maintenable, I really like it. The second one is harder to read, at least for me, and requires a c++ skill level maybe over the average, surely over my one. Not something everyone is able to write. Just learned somethin new, thanks
– Gian Paolo
5 hours ago
add a comment |
up vote
2
down vote
Consider translating your tables as directly as possible into your program. Drive the program based off the table, instead of mimicing it with logic.
template<class T0>
auto is_any_of( T0 const& t0, std::initializer_list<T0> il ) {
for (auto&& x:il)
if (x==t0) return true;
return false;
}
now
if (is_any_of(
std::make_tuple(bValue1, bValue2, bValue3, bValue4),
{
{true, true, true, true},
{true, true, true, false},
{true, false, false, false}
}
))
this directly as possible encodes your truth table into the compiler.
Live example.
You could also use std::any_of
directly:
using entry = std::array<bool, 4>;
constexpr entry acceptable =
{
{true, true, true, true},
{true, true, true, false},
{true, false, false, false}
};
if (std::any_of( begin(acceptable), end(acceptable), [&](auto&&x){
return entry{bValue1, bValue2, bValue3, bValue4} == x;
}) {
}
the compiler can inline the code, and eliminate any iteration and build its own logic for you. Meanwhile, your code reflects exactly how you concieved of the problem.
The first version is so easy to read and so maintenable, I really like it. The second one is harder to read, at least for me, and requires a c++ skill level maybe over the average, surely over my one. Not something everyone is able to write. Just learned somethin new, thanks
– Gian Paolo
5 hours ago
add a comment |
up vote
2
down vote
up vote
2
down vote
Consider translating your tables as directly as possible into your program. Drive the program based off the table, instead of mimicing it with logic.
template<class T0>
auto is_any_of( T0 const& t0, std::initializer_list<T0> il ) {
for (auto&& x:il)
if (x==t0) return true;
return false;
}
now
if (is_any_of(
std::make_tuple(bValue1, bValue2, bValue3, bValue4),
{
{true, true, true, true},
{true, true, true, false},
{true, false, false, false}
}
))
this directly as possible encodes your truth table into the compiler.
Live example.
You could also use std::any_of
directly:
using entry = std::array<bool, 4>;
constexpr entry acceptable =
{
{true, true, true, true},
{true, true, true, false},
{true, false, false, false}
};
if (std::any_of( begin(acceptable), end(acceptable), [&](auto&&x){
return entry{bValue1, bValue2, bValue3, bValue4} == x;
}) {
}
the compiler can inline the code, and eliminate any iteration and build its own logic for you. Meanwhile, your code reflects exactly how you concieved of the problem.
Consider translating your tables as directly as possible into your program. Drive the program based off the table, instead of mimicing it with logic.
template<class T0>
auto is_any_of( T0 const& t0, std::initializer_list<T0> il ) {
for (auto&& x:il)
if (x==t0) return true;
return false;
}
now
if (is_any_of(
std::make_tuple(bValue1, bValue2, bValue3, bValue4),
{
{true, true, true, true},
{true, true, true, false},
{true, false, false, false}
}
))
this directly as possible encodes your truth table into the compiler.
Live example.
You could also use std::any_of
directly:
using entry = std::array<bool, 4>;
constexpr entry acceptable =
{
{true, true, true, true},
{true, true, true, false},
{true, false, false, false}
};
if (std::any_of( begin(acceptable), end(acceptable), [&](auto&&x){
return entry{bValue1, bValue2, bValue3, bValue4} == x;
}) {
}
the compiler can inline the code, and eliminate any iteration and build its own logic for you. Meanwhile, your code reflects exactly how you concieved of the problem.
edited 7 hours ago
answered 7 hours ago
Yakk - Adam Nevraumont
179k19187367
179k19187367
The first version is so easy to read and so maintenable, I really like it. The second one is harder to read, at least for me, and requires a c++ skill level maybe over the average, surely over my one. Not something everyone is able to write. Just learned somethin new, thanks
– Gian Paolo
5 hours ago
add a comment |
The first version is so easy to read and so maintenable, I really like it. The second one is harder to read, at least for me, and requires a c++ skill level maybe over the average, surely over my one. Not something everyone is able to write. Just learned somethin new, thanks
– Gian Paolo
5 hours ago
The first version is so easy to read and so maintenable, I really like it. The second one is harder to read, at least for me, and requires a c++ skill level maybe over the average, surely over my one. Not something everyone is able to write. Just learned somethin new, thanks
– Gian Paolo
5 hours ago
The first version is so easy to read and so maintenable, I really like it. The second one is harder to read, at least for me, and requires a c++ skill level maybe over the average, surely over my one. Not something everyone is able to write. Just learned somethin new, thanks
– Gian Paolo
5 hours ago
add a comment |
up vote
1
down vote
Every answer is overly complex and difficult to read. The best solution to this is a switch()
statement. It is both readable and makes adding/modifying additional simple. Compilers are good at optimising switch()
statements too.
switch( (bValue4 << 3) | (bValue3 << 2) | (bValue2 << 1) | (bValue1) )
{
case 0b1111:
// scenario 1
break;
case 0b0111:
// scenario 2
break;
case 0b0001:
// scenario 3
break;
default:
// fault condition
break;
}
You can of course use constants and OR them together in the case
statements for even greater readability.
add a comment |
up vote
1
down vote
Every answer is overly complex and difficult to read. The best solution to this is a switch()
statement. It is both readable and makes adding/modifying additional simple. Compilers are good at optimising switch()
statements too.
switch( (bValue4 << 3) | (bValue3 << 2) | (bValue2 << 1) | (bValue1) )
{
case 0b1111:
// scenario 1
break;
case 0b0111:
// scenario 2
break;
case 0b0001:
// scenario 3
break;
default:
// fault condition
break;
}
You can of course use constants and OR them together in the case
statements for even greater readability.
add a comment |
up vote
1
down vote
up vote
1
down vote
Every answer is overly complex and difficult to read. The best solution to this is a switch()
statement. It is both readable and makes adding/modifying additional simple. Compilers are good at optimising switch()
statements too.
switch( (bValue4 << 3) | (bValue3 << 2) | (bValue2 << 1) | (bValue1) )
{
case 0b1111:
// scenario 1
break;
case 0b0111:
// scenario 2
break;
case 0b0001:
// scenario 3
break;
default:
// fault condition
break;
}
You can of course use constants and OR them together in the case
statements for even greater readability.
Every answer is overly complex and difficult to read. The best solution to this is a switch()
statement. It is both readable and makes adding/modifying additional simple. Compilers are good at optimising switch()
statements too.
switch( (bValue4 << 3) | (bValue3 << 2) | (bValue2 << 1) | (bValue1) )
{
case 0b1111:
// scenario 1
break;
case 0b0111:
// scenario 2
break;
case 0b0001:
// scenario 3
break;
default:
// fault condition
break;
}
You can of course use constants and OR them together in the case
statements for even greater readability.
answered 15 hours ago
shogged
1586
1586
add a comment |
add a comment |
up vote
1
down vote
First, assuming you can only modify the scenario check, I would focus on readability and just wrap the check in a function so that you can just call if(ScenarioA())
.
Now, assuming you actually want/need to optimize this, I would recommend converting the tightly linked Booleans into constant integers, and using bit operators on them
public class Options {
public const bool A = 2; // 0001
public const bool B = 4; // 0010
public const bool C = 16;// 0100
public const bool D = 32;// 1000
//public const bool N = 2^n; (up to n=32)
}
...
public isScenario3(int options) {
int s3 = Options.A | Options.B | Options.C;
// for true if only s3 options are set
return options == s3;
// for true if s3 options are set
// return options & s3 == s3
}
This makes expressing the scenarios as easy as listing what is part of it, allows you to use a switch statement to jump to the right condition, and confuse fellow developers who have not seen this before. (C# RegexOptions uses this pattern for setting flags, I don't know if there is a c++ library example)
add a comment |
up vote
1
down vote
First, assuming you can only modify the scenario check, I would focus on readability and just wrap the check in a function so that you can just call if(ScenarioA())
.
Now, assuming you actually want/need to optimize this, I would recommend converting the tightly linked Booleans into constant integers, and using bit operators on them
public class Options {
public const bool A = 2; // 0001
public const bool B = 4; // 0010
public const bool C = 16;// 0100
public const bool D = 32;// 1000
//public const bool N = 2^n; (up to n=32)
}
...
public isScenario3(int options) {
int s3 = Options.A | Options.B | Options.C;
// for true if only s3 options are set
return options == s3;
// for true if s3 options are set
// return options & s3 == s3
}
This makes expressing the scenarios as easy as listing what is part of it, allows you to use a switch statement to jump to the right condition, and confuse fellow developers who have not seen this before. (C# RegexOptions uses this pattern for setting flags, I don't know if there is a c++ library example)
add a comment |
up vote
1
down vote
up vote
1
down vote
First, assuming you can only modify the scenario check, I would focus on readability and just wrap the check in a function so that you can just call if(ScenarioA())
.
Now, assuming you actually want/need to optimize this, I would recommend converting the tightly linked Booleans into constant integers, and using bit operators on them
public class Options {
public const bool A = 2; // 0001
public const bool B = 4; // 0010
public const bool C = 16;// 0100
public const bool D = 32;// 1000
//public const bool N = 2^n; (up to n=32)
}
...
public isScenario3(int options) {
int s3 = Options.A | Options.B | Options.C;
// for true if only s3 options are set
return options == s3;
// for true if s3 options are set
// return options & s3 == s3
}
This makes expressing the scenarios as easy as listing what is part of it, allows you to use a switch statement to jump to the right condition, and confuse fellow developers who have not seen this before. (C# RegexOptions uses this pattern for setting flags, I don't know if there is a c++ library example)
First, assuming you can only modify the scenario check, I would focus on readability and just wrap the check in a function so that you can just call if(ScenarioA())
.
Now, assuming you actually want/need to optimize this, I would recommend converting the tightly linked Booleans into constant integers, and using bit operators on them
public class Options {
public const bool A = 2; // 0001
public const bool B = 4; // 0010
public const bool C = 16;// 0100
public const bool D = 32;// 1000
//public const bool N = 2^n; (up to n=32)
}
...
public isScenario3(int options) {
int s3 = Options.A | Options.B | Options.C;
// for true if only s3 options are set
return options == s3;
// for true if s3 options are set
// return options & s3 == s3
}
This makes expressing the scenarios as easy as listing what is part of it, allows you to use a switch statement to jump to the right condition, and confuse fellow developers who have not seen this before. (C# RegexOptions uses this pattern for setting flags, I don't know if there is a c++ library example)
answered 11 hours ago
Tezra
4,90821042
4,90821042
add a comment |
add a comment |
up vote
1
down vote
It depends on what they represent.
For example if 1 is a key, and 2 and 3 are two people who must agree (except if they agree on NOT
they need a third person - 4 - to confirm) the most readable might be:
1 &&
(
(2 && 3)
||
((!2 && !3) && !4)
)
1
You might be right, but using numbers to illustrate your point detracts from your answer. Try using descriptive names.
– jxh
yesterday
1
@jxh Those are the numbers OP used. I just removed thebValue
.
– ispiro
yesterday
Left as generic booleans, your construction does not improve readability. Readability only improves if the booleans are given descriptive names that motivates the construction you suggest.
– jxh
yesterday
add a comment |
up vote
1
down vote
It depends on what they represent.
For example if 1 is a key, and 2 and 3 are two people who must agree (except if they agree on NOT
they need a third person - 4 - to confirm) the most readable might be:
1 &&
(
(2 && 3)
||
((!2 && !3) && !4)
)
1
You might be right, but using numbers to illustrate your point detracts from your answer. Try using descriptive names.
– jxh
yesterday
1
@jxh Those are the numbers OP used. I just removed thebValue
.
– ispiro
yesterday
Left as generic booleans, your construction does not improve readability. Readability only improves if the booleans are given descriptive names that motivates the construction you suggest.
– jxh
yesterday
add a comment |
up vote
1
down vote
up vote
1
down vote
It depends on what they represent.
For example if 1 is a key, and 2 and 3 are two people who must agree (except if they agree on NOT
they need a third person - 4 - to confirm) the most readable might be:
1 &&
(
(2 && 3)
||
((!2 && !3) && !4)
)
It depends on what they represent.
For example if 1 is a key, and 2 and 3 are two people who must agree (except if they agree on NOT
they need a third person - 4 - to confirm) the most readable might be:
1 &&
(
(2 && 3)
||
((!2 && !3) && !4)
)
edited 8 hours ago
answered yesterday
ispiro
12.8k1979178
12.8k1979178
1
You might be right, but using numbers to illustrate your point detracts from your answer. Try using descriptive names.
– jxh
yesterday
1
@jxh Those are the numbers OP used. I just removed thebValue
.
– ispiro
yesterday
Left as generic booleans, your construction does not improve readability. Readability only improves if the booleans are given descriptive names that motivates the construction you suggest.
– jxh
yesterday
add a comment |
1
You might be right, but using numbers to illustrate your point detracts from your answer. Try using descriptive names.
– jxh
yesterday
1
@jxh Those are the numbers OP used. I just removed thebValue
.
– ispiro
yesterday
Left as generic booleans, your construction does not improve readability. Readability only improves if the booleans are given descriptive names that motivates the construction you suggest.
– jxh
yesterday
1
1
You might be right, but using numbers to illustrate your point detracts from your answer. Try using descriptive names.
– jxh
yesterday
You might be right, but using numbers to illustrate your point detracts from your answer. Try using descriptive names.
– jxh
yesterday
1
1
@jxh Those are the numbers OP used. I just removed the
bValue
.– ispiro
yesterday
@jxh Those are the numbers OP used. I just removed the
bValue
.– ispiro
yesterday
Left as generic booleans, your construction does not improve readability. Readability only improves if the booleans are given descriptive names that motivates the construction you suggest.
– jxh
yesterday
Left as generic booleans, your construction does not improve readability. Readability only improves if the booleans are given descriptive names that motivates the construction you suggest.
– jxh
yesterday
add a comment |
up vote
1
down vote
If (!bValue1 || (bValue2 != bValue3) || (!bValue4 && bValue2))
{
// you have a problem
}
- b1 must always be true
- b2 must always equal b3
- and b4 cannot be false
if b2 (and b3) are true
simple
New contributor
add a comment |
up vote
1
down vote
If (!bValue1 || (bValue2 != bValue3) || (!bValue4 && bValue2))
{
// you have a problem
}
- b1 must always be true
- b2 must always equal b3
- and b4 cannot be false
if b2 (and b3) are true
simple
New contributor
add a comment |
up vote
1
down vote
up vote
1
down vote
If (!bValue1 || (bValue2 != bValue3) || (!bValue4 && bValue2))
{
// you have a problem
}
- b1 must always be true
- b2 must always equal b3
- and b4 cannot be false
if b2 (and b3) are true
simple
New contributor
If (!bValue1 || (bValue2 != bValue3) || (!bValue4 && bValue2))
{
// you have a problem
}
- b1 must always be true
- b2 must always equal b3
- and b4 cannot be false
if b2 (and b3) are true
simple
New contributor
New contributor
answered 4 hours ago
Owen Meyer
111
111
New contributor
New contributor
add a comment |
add a comment |
up vote
1
down vote
Doing bitwise operation looks very clean and understandable.
int bitwise = (bValue4 << 3) | (bValue3 << 2) | (bValue2 << 1) | (bValue1);
if (bitwise == 0b1111 || bitwise == 0b0111 || bitwise == 0b0001)
{
//satisfying condition
}
add a comment |
up vote
1
down vote
Doing bitwise operation looks very clean and understandable.
int bitwise = (bValue4 << 3) | (bValue3 << 2) | (bValue2 << 1) | (bValue1);
if (bitwise == 0b1111 || bitwise == 0b0111 || bitwise == 0b0001)
{
//satisfying condition
}
add a comment |
up vote
1
down vote
up vote
1
down vote
Doing bitwise operation looks very clean and understandable.
int bitwise = (bValue4 << 3) | (bValue3 << 2) | (bValue2 << 1) | (bValue1);
if (bitwise == 0b1111 || bitwise == 0b0111 || bitwise == 0b0001)
{
//satisfying condition
}
Doing bitwise operation looks very clean and understandable.
int bitwise = (bValue4 << 3) | (bValue3 << 2) | (bValue2 << 1) | (bValue1);
if (bitwise == 0b1111 || bitwise == 0b0111 || bitwise == 0b0001)
{
//satisfying condition
}
answered 4 hours ago
Simonare
385111
385111
add a comment |
add a comment |
up vote
-2
down vote
use bit field:
unoin {
struct {
bool b1: 1;
bool b2: 1;
bool b3: 1;
bool b4: 1;
} b;
int i;
} u;
// set:
u.b.b1=true;
...
// test
if (u.i == 0x0f) {...}
if (u.i == 0x0e) {...}
if (u.i == 0x08) {...}
2
This causes UB due to accessing an inactive union field.
– HolyBlackCat
13 hours ago
Formally it's UB in C++, you can't set one member of union and read from another. Technically it might be better to implement templated getterssetters for bits of integral value.
– Swift - Friday Pie
13 hours ago
I think the behavior would shift to Implementation-Defined if one were to convert the union's address to anunsigned char*
, though I think simply using something like((((flag4 <<1) | flag3) << 1) | flag2) << 1) | flag1
would probably be more efficient.
– supercat
7 hours ago
add a comment |
up vote
-2
down vote
use bit field:
unoin {
struct {
bool b1: 1;
bool b2: 1;
bool b3: 1;
bool b4: 1;
} b;
int i;
} u;
// set:
u.b.b1=true;
...
// test
if (u.i == 0x0f) {...}
if (u.i == 0x0e) {...}
if (u.i == 0x08) {...}
2
This causes UB due to accessing an inactive union field.
– HolyBlackCat
13 hours ago
Formally it's UB in C++, you can't set one member of union and read from another. Technically it might be better to implement templated getterssetters for bits of integral value.
– Swift - Friday Pie
13 hours ago
I think the behavior would shift to Implementation-Defined if one were to convert the union's address to anunsigned char*
, though I think simply using something like((((flag4 <<1) | flag3) << 1) | flag2) << 1) | flag1
would probably be more efficient.
– supercat
7 hours ago
add a comment |
up vote
-2
down vote
up vote
-2
down vote
use bit field:
unoin {
struct {
bool b1: 1;
bool b2: 1;
bool b3: 1;
bool b4: 1;
} b;
int i;
} u;
// set:
u.b.b1=true;
...
// test
if (u.i == 0x0f) {...}
if (u.i == 0x0e) {...}
if (u.i == 0x08) {...}
use bit field:
unoin {
struct {
bool b1: 1;
bool b2: 1;
bool b3: 1;
bool b4: 1;
} b;
int i;
} u;
// set:
u.b.b1=true;
...
// test
if (u.i == 0x0f) {...}
if (u.i == 0x0e) {...}
if (u.i == 0x08) {...}
answered 13 hours ago
hedzr
9723
9723
2
This causes UB due to accessing an inactive union field.
– HolyBlackCat
13 hours ago
Formally it's UB in C++, you can't set one member of union and read from another. Technically it might be better to implement templated getterssetters for bits of integral value.
– Swift - Friday Pie
13 hours ago
I think the behavior would shift to Implementation-Defined if one were to convert the union's address to anunsigned char*
, though I think simply using something like((((flag4 <<1) | flag3) << 1) | flag2) << 1) | flag1
would probably be more efficient.
– supercat
7 hours ago
add a comment |
2
This causes UB due to accessing an inactive union field.
– HolyBlackCat
13 hours ago
Formally it's UB in C++, you can't set one member of union and read from another. Technically it might be better to implement templated getterssetters for bits of integral value.
– Swift - Friday Pie
13 hours ago
I think the behavior would shift to Implementation-Defined if one were to convert the union's address to anunsigned char*
, though I think simply using something like((((flag4 <<1) | flag3) << 1) | flag2) << 1) | flag1
would probably be more efficient.
– supercat
7 hours ago
2
2
This causes UB due to accessing an inactive union field.
– HolyBlackCat
13 hours ago
This causes UB due to accessing an inactive union field.
– HolyBlackCat
13 hours ago
Formally it's UB in C++, you can't set one member of union and read from another. Technically it might be better to implement templated getterssetters for bits of integral value.
– Swift - Friday Pie
13 hours ago
Formally it's UB in C++, you can't set one member of union and read from another. Technically it might be better to implement templated getterssetters for bits of integral value.
– Swift - Friday Pie
13 hours ago
I think the behavior would shift to Implementation-Defined if one were to convert the union's address to an
unsigned char*
, though I think simply using something like ((((flag4 <<1) | flag3) << 1) | flag2) << 1) | flag1
would probably be more efficient.– supercat
7 hours ago
I think the behavior would shift to Implementation-Defined if one were to convert the union's address to an
unsigned char*
, though I think simply using something like ((((flag4 <<1) | flag3) << 1) | flag2) << 1) | flag1
would probably be more efficient.– supercat
7 hours ago
add a comment |
up vote
-2
down vote
A simple approach is finding the answer that you think are acceptable.
Yes = (boolean1 && boolean2 && boolean3 && boolean4) + + ...
Now if possible simplify the equation using boolean algebra.
like in this case, acceptable1 and 2 combine to (boolean1 && boolean2 && boolean3)
Hence the final answer is
(boolean1 && boolean2 && boolean3) + ((boolean1 && !boolean2 && !boolean3 && !boolean4)
P.S. + means OR operator.
add a comment |
up vote
-2
down vote
A simple approach is finding the answer that you think are acceptable.
Yes = (boolean1 && boolean2 && boolean3 && boolean4) + + ...
Now if possible simplify the equation using boolean algebra.
like in this case, acceptable1 and 2 combine to (boolean1 && boolean2 && boolean3)
Hence the final answer is
(boolean1 && boolean2 && boolean3) + ((boolean1 && !boolean2 && !boolean3 && !boolean4)
P.S. + means OR operator.
add a comment |
up vote
-2
down vote
up vote
-2
down vote
A simple approach is finding the answer that you think are acceptable.
Yes = (boolean1 && boolean2 && boolean3 && boolean4) + + ...
Now if possible simplify the equation using boolean algebra.
like in this case, acceptable1 and 2 combine to (boolean1 && boolean2 && boolean3)
Hence the final answer is
(boolean1 && boolean2 && boolean3) + ((boolean1 && !boolean2 && !boolean3 && !boolean4)
P.S. + means OR operator.
A simple approach is finding the answer that you think are acceptable.
Yes = (boolean1 && boolean2 && boolean3 && boolean4) + + ...
Now if possible simplify the equation using boolean algebra.
like in this case, acceptable1 and 2 combine to (boolean1 && boolean2 && boolean3)
Hence the final answer is
(boolean1 && boolean2 && boolean3) + ((boolean1 && !boolean2 && !boolean3 && !boolean4)
P.S. + means OR operator.
answered 13 hours ago
Rupesh
175
175
add a comment |
add a comment |
Thanks for contributing an answer to Stack Overflow!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Some of your past answers have not been well-received, and you're in danger of being blocked from answering.
Please pay close attention to the following guidance:
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53591559%2fhow-to-improve-logic-to-check-whether-4-boolean-values-match-some-cases%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
5
I would use a table instead of complex
if
statement. Additionally, as these are boolean flags, you can model each scenario as a constant and check against it.– Zdeslav Vojkovic
yesterday
3
if (!((bValue1 && bValue2 && bValue3) || (bValue1 && !bValue2 && !bValue3 && !bValue4)))
– mch
yesterday
8
what are the scenarios actually? Often things get much simpler if you just give stuff proper names, eg
bool scenario1 = bValue1 && bValue2 && bValue3 && bValue4;
– user463035818
yesterday
2
Using meaningful names, you can extract each complex condition into a method and call that method in if condition. It would be much more readable and maintainable. e.g. Take a look at the example provided in the link.refactoring.guru/decompose-conditional
– Hardik Modha
yesterday
2
@HardikModha Yeah, I see how I might be able to implement that in the code I have now for modular clarity. Thank you.
– Andrew Truckle
yesterday