# nice way to format long conditional statements?

Is there a cleaner way to write long conditionals such as this:

``````  // A = (round(tIn) > userSet[setPnt]+1)
// B = (round(tIn) < userSet[setPnt])
// C = vOpen
// D = (tOut > tIn)
// E = ((millis() - tmVclosed) > tmIntrvl * 60 * 1000)
//
// from the truth table...
// if ((A || (!A && !B)) && C && D) || (B && C) close valves and note millis()
// else if ((A && !C && !D) || (A && !C && D && E)) open valves

if ((((round(tIn) > userSet[setPnt]+1) || (!(round(tIn) > userSet[setPnt]+1) && !(round(tIn) < userSet[setPnt]))) && vOpen && (tOut > tIn)) || ((round(tIn) < userSet[setPnt]) && vOpen)) {
closeValves();
tmVclosed=millis();
vOpen=false;
}
else if ((((round(tIn) > userSet[setPnt]+1) && !vOpen && !(tOut > tIn)) || ((round(tIn) > userSet[setPnt]+1) && !vOpen && (tOut > tIn) && ((millis() - tmVclosed) > tmIntrvl * 60 * 1000)))) {
openValves();
vOpen=true;
}
``````

Hum. Well, I post, and then immediately find an answer in here.

(Stretching them out over multiple lines.)

Open to other suggestions...

Well, the comments describing the logic could be macros so the code becomes identical to the comments.

``````#define A (round(tIn) > userSet[setPnt]+1)
...
// from the truth table...
if ((A || (!A && !B)) && C && D) || (B && C) { //close valves and note millis()
``````

But macros used like this can be dangerous with side-effects. I would also give them meaningful names instead of ABCD.

Assuming all of those variables are global, you can use functions, once again meaningful names are preferred but I don’t know what your code means so I have to use ABCD:

``````boolean A() {
return (round(tIn) > userSet[setPnt]+1);
}
...
// from the truth table...
if ((A() || (!A() && !B())) && C() && D()) || (B() && C()) { //close valves and note millis()
``````

Or wrap it all into a pair of functions:

``````boolean timeToCloseValves() {
return  ((((round(tIn) > userSet[setPnt]+1)
|| (!(round(tIn) > userSet[setPnt]+1) && !(round(tIn) < userSet[setPnt])))
&& vOpen
&& (tOut > tIn))
|| ((round(tIn) < userSet[setPnt]) && vOpen));
}

...

if (timeToCloseValves()) {
closeValves();
tmVclosed=millis();
vOpen=false;
}
``````

@DaveEvans:
Looks like you've been messing around with Karnaugh maps too long.

Of course, you can -- and in your case, probably should -- use nested "if" statements.

``````if (well_named_predicate (...))
well_named_function1 () ;
else
well_named_function2 () ;
``````

Err on the side of more smaller functions, rather than a few complicated ones and name them
carefully - then the code is readable and maintainable and easy to adapt to changing requirements.
Most functions should be 10 lines or fewer. There are times when you need a long list of stuff
in a function, but it should be the exception not the rule.

Use switch/case whenever its possible, its more concise for a whole set of possible values to
match.

instead of functions use some local bool variables (your comment becomes code)

``````bool A = round(tIn) > userSet[setPnt]+1;
bool B = round(tIn) < userSet[setPnt];
bool C = vOpen;
bool D = tOut > tIn;
bool E = (millis() - tmVclosed) > tmIntrvl * 60 * 1000;

if (((A || (!A && !B)) && C && D) || (B && C))
{
closeValves();
tmVclosed=millis();
vOpen=false;
}
else if ((A && !C && !D) || (A && !C && D && E))
{
openValves();
vOpen=true;
}
``````

(((A || (!A && !B)) && C && D) || (B && C))

can be written as

CD + BC => ((C && D) || (B && C))

completely removing expression A

(please check if I didn’t make mistakes :))

Personally I prefer the style in Reply #4. I have no confidence in my ability to get something like this
if (((A || (!A && !B)) && C && D) || (B && C))
correct. And even if I got it correct I know that I would need to spend a long time trying to convince myself it is correct when I return to the program after an absence of a few weeks.

...R

Thanks to everyone for their helpful replies.

You’re right! And that’s a pretty cool website. I remembered truth tables from my (long ago) college days, but not Karnaugh maps. By looking at my truth table, I should have realized that (A or (!A and !B)) = 1, but the CalPoly tool shows it clearly. (And, if I’d remembered the distributive law, it would have been obvious, too…)

robtillaart:
instead of functions use some local bool variables (your comment becomes code)

``````bool A = round(tIn) > userSet[setPnt]+1;
``````

bool B = round(tIn) < userSet[setPnt];
bool C = vOpen;
bool D = tOut > tIn;
bool E = (millis() - tmVclosed) > tmIntrvl * 60 * 1000;

if (((A || (!A && !B)) && C && D) || (B && C))
{
closeValves();
tmVclosed=millis();
vOpen=false;
}
else if ((A && !C && !D) || (A && !C && D && E))
{
openValves();
vOpen=true;
}

``````

**(((A || (!A && !B)) && C && D) || (B && C))**

can be written as

**CD + BC => ((C && D) || (B && C))**

completely removing expression A

(please check if I didn't make mistakes :))``````

It's been said that 80% of the cost of software development is in the testing and debugging. Anything you can do to make your code easier to read is usually a good thing. (There are probably times when "easier-to-read" causes a severe performance hit.)

Personally, I find cascading if statements much harder to understand than a switch. I find complex AND and OR statements like those discussed here difficult to debug. Often during testing, I break these multiple conditionals out into single statements, one after the other, until I'm sure I have them correct. I then comment out the single statement version and build the "complex" one. That way, if I come back six months later, I can read the commented code and "get up to speed" again a tad bit quicker than otherwise. If the final analysis, it's partly a question of your style and how your brain views such complexity.

@econjack:
I guess it depends on your hrair limit.