2

I have reasonably complicated logic for if statements. I am currently using:

if(numerical_evaluation) {

    if ((!boolOne && boolTwo) || !boolThree){
        //do stuff
    }
}

where boolOne, boolTwo, and boolThree are logical operations (perhaps x < y or 'myObject->getBool' etc).

I do not know a better way to make this easily readable without nesting the || condition within a third if statement.

The reason I am having difficulty is that the or operator makes it such that it seems a third if statement might be warranted.

One option would be to do this.

Or, I could do something like

if(x <= y) {

    bool boolFour = false;
    if ((!boolOne && boolTwo))
        boolFour = true;

    if (boolFour || !boolThree){
        //do stuff
    }
}

Maybe even make a separate function to try to validate everything or combinations into a single return value?

Alternatively, I could attempt to restructure the code in such a way this is not required, which may require significant amounts of time to do.

My question: What is the best way to format complicated if questions - which include more complicated evaluations than just if (!A && B && C) variations? It seems things get hopelessly unreadable (especially when you have complicated evaluations for boolOne, boolTwo, etc) when you include || statements in combination with && statements into one line. Do the same principles from - Best way to format if statement with multiple conditions - apply here too or are there fundamental differences when using a variety of logical operators?

Community
  • 1
  • 1
enderland
  • 13,825
  • 17
  • 98
  • 152
  • This is probably better suited for [Programmers.SE](http://programmers.stackexchange.com/). – ildjarn Aug 27 '12 at 22:00
  • Why unreadable? This is perfectly readable... – ForEveR Aug 27 '12 at 22:00
  • This is subjective and will probably be closed. Personally I like to encapsulate logic into descriptive functions, so the final statement is like, `if(HasAccess() && IsEditable()) { ...`. – tenfour Aug 27 '12 at 22:01

6 Answers6

13

This:

bool boolFour = false;
if ((!boolOne && boolTwo))
    boolFour = true;

can be expressed much more clearly as:

bool const boolFour = !boolOne && boolTwo;

By giving boolFour a good, descriptive name, this approach of breaking down complex expressions and naming subexpressions can make code much more readable, much easier to understand, and much easier to debug.

If a complex expression is used in multiple places, a function should be used to encapsulate the common logic. If the expression is only used in one place, though, it is preferable to break up the expression locally and use named const variables, to keep the logic close to where it is used.

James McNellis
  • 348,265
  • 75
  • 913
  • 977
  • 3
    A combination of descriptive naming and [De Morgan's Law](http://en.wikipedia.org/wiki/DeMorgan%27s_Law) can work wonders on the readability of an expression. – Mark Ransom Aug 27 '12 at 22:06
  • This is a really good approach, which, for some reason, I never thought to take. – enderland Aug 27 '12 at 22:08
4

Write a helper function that encapsulates the combined boolean checks. E.g:

bool isEligibleForReduction(int age) { return age < 12 || age >= 60; }
StackedCrooked
  • 34,653
  • 44
  • 154
  • 278
  • Just an off topic (and fairly useless) comment here... why not `age < 12 || age > 59` or for that matter `age <= 11 || age >=60`? – Ed S. Aug 27 '12 at 22:05
  • Because I assume that in human language the rules state the children younger than 12 and adults 60 or older are eligible for a reduction. Your suggestion is correct as well of course. – StackedCrooked Aug 27 '12 at 22:07
  • Yes it is strange no? Why not stick with one form of comparison? Oh well. – Ed S. Aug 27 '12 at 22:08
0

if possible I would usually do something like:

if(numerical_evaluation) { 

    meaningful_name = (!boolOne && boolTwo);
    other_meaningful_name = !boolThree;
    if (meaningful_name || other_meaningful_name){ 
        //do stuff 
    } 
} 
tletnes
  • 1,958
  • 10
  • 30
0
if (!numerical_evaluation) {
    // nothing to do.
} else if (!boolOne && boolTwo || !boolThree) {
    // do whatever
}

Of course, as others have said, boolOne, boolTwo, and boolThree aren't very helpful names.

Pete Becker
  • 74,985
  • 8
  • 76
  • 165
0

The best way is good use of white space.

if( numerical_evaluation &&
    (
        (!boolOne && boolTwo) ||
        !boolThree
    )
  ) {
        //do stuff
}

It's not exactly pretty, but it's easy enough to follow. You can also use a function to hide the if logic.

bool my_test( int numerical_evaluation, bool boolOne, bool boolTwo, bool boolThree ) {
    return
        numerical_evaluation &&
        (
            (!boolOne && boolTwo) ||
            !boolThree
        );
}

if( my_test( numerical_evaluation, boolOne, boolTwo, boolThree ) ) {
    // do stuff
}

Remember, when it's not a trivial case, use comments to give people an idea of what you're testing (no need for comments that just explain C++ syntax). Even if they can read your if logic just fine, it lets them double check it. Good comments can quickly give people an overview of your program and much of the logic, without reading unnecessary details or any code.

jbo5112
  • 824
  • 1
  • 11
  • 18
  • 2
    Oh my. No, no, no. This is not better. I've had the pleasure of maintaining a 5,000 line source file that made heavy use of complex, "well-formatted" expressions like this, and it was a disaster. It was near impossible to debug. Once you have even two or three levels of mixing of operators, it's very hard to keep track of which operators go with which operands. Just break the expression up into multiple, well-named variables representing each subexpression. This way the code is self-documenting, easier to debug, and easier to understand. – James McNellis Aug 27 '12 at 23:18
  • That method has been suggested. It doesn't work well with boolean algebra and logic reduction, which can lead the nightmare of poor code w/ absurdly too many subexpressions stored into variables. With networked systems, I don't always get the luxury of using such variables. I wanted to add this to the conversation. It takes a little practice, but with good names I can read this more easily. When I'm not doing admin work, I personally maintain ~50,000 lines of C++ code, and I only use extra variables in performance cases. I've lost 10,000x more time to Oracle bugs than mine. – jbo5112 Sep 11 '12 at 03:28
0

Although it was a performance suggestion, complicated boolean expressions are sometimes better represented as a table lookup.

Something complicated like:

if( (a && !c) || (a && b && c )) 
{
    category = 1;
}
else if( (b && !a ) || (a && c && !b )
{
   category = 2;
}
else if( c && !a && !b ) 
{
   category = 3;
}
else
{
    category = 0;
}

Becomes:

static int categoryTable[2][2][2] = {
    // !b!c    !bc    b!c    bc
    0,         3,     2,     2,      // !a
    1,         2,     1,     1       // a
};
... 
category = categoryTable[a][b][c];

Code Complete 2 page 614 & 615, "Substitute Table Lookups for Complicated Expressions".

oz10
  • 153,307
  • 27
  • 93
  • 128