Fairly often I find myself writing complex boolean expressions. For example, let’s say I’m writing a function in a controller that renders a page, and I want to check whether the current user is authorized to view that page. Here’s some code:
if ((!($a || $b)) && $c != $d) || !$e) {
// Do something.
}
Aside from the variable names, that’s real code. There are two things about that code that to me greatly hinder readability. I rewrote it as essentially the following:
if ($x != $y || !$z || !$w) {
// Do something.
}
The second code has two advantages. First, no internal parentheses are needed, and second, there’s no mixture of “and” and “or”. Whenever I can, I like to use only one boolean operator in a complex boolean expression, because sorting out different operators can be confusing. In the second example, I know that if any of the expressions evaluates as true, then the code in the block will be invoked. When you mix operators, things aren’t so straightforward. And not requiring parentheses is related. When you mix operators, precedence comes into play, and you can use parentheses to manage precedence. So as soon as you bring either of them to the party, you can no longer read your expressions from right to left.
This just occurred to me so there’s probably something I’m missing, but for the time being, I’m going to try to be disciplined about sticking to one boolean operator at a time in cases like this. (And yes, I’m not counting the “nots”.)
November 10, 2009 at 1:37 pm
I like the idea, but the second example is too clean. You have jettisoned a variable, and the problematic one at that. I might be missing a transformation, but I think you can’t get to “all or” or “all and” with the “|| $e” bit of logic intact. The cleanest I got was…
if ( !( ($a || $b || $c=$d) && $e ) ) {…
I like how $a, $b, $c, and $d came out, but integrating the $e logic still gets ugly. Maybe you pushed that to an inner “if” statement.
November 10, 2009 at 2:24 pm
You can replace AND with nested if statements.
if (!($a || $b)) { if ($c != $d) { doSomething(); } } else if (!$e) { doSomething(); }
In contexts where you can do an early break, continue, or return, they don’t even need to be nested. Having lots of small functions allows for this.
November 10, 2009 at 2:46 pm
In the second example with the dropped variable I actually figured out another way to get the same information using different functions (and I would have written the necessary functions if needed.)
Using the same variables it would have been more like:
I just arrived at the “!$a || !$b” part another way.
November 10, 2009 at 4:02 pm
I find them both pretty unreadable (though the variable names are probably the main cause there).
I usually either nest ifs (not always easy if you have else clauses) or extract the tests out into small static methods with descriptive names.
Tracking the effect of boolean logic is not something humans do very well. Computers, sure.
November 10, 2009 at 4:06 pm
Yes, in real life all of the variables above are actually method calls.
November 10, 2009 at 4:46 pm
Fair enough.
One thing I miss very much from Perl is the “and”, “or”, and “not” operators – for non-Perl programmers, those are the literal strings, not “&&”, “||” and “!” – which made boolean expressions much easier to read and also had saner precedence than the C versions.
November 10, 2009 at 5:37 pm
Rafe, this is interesting, but I think if what you are trying to do is improve readability, you are missing the real win, which is functionalizing the permission logic. With that said, maybe that is not so easy (lots of variables to pass?).
I also think this post is a little bit of a trap. There is a tendency to treat boolean expressions only as a logic problem, while ignoring meaning or intent. I generally have better success when I treat the expressions as sentences, and figure out how to make them read correctly and understandably.
I’m not quite sure how to explain this, but because this post is framed only in terms of context-free placeholder variables, I think it’s actually impossible to make a meaningful evaluation of the code structure. We are limited to only evaluating the abstract logical equivalences. This is fun, but I think if you want to talk about how to code well you need to talk about meaning and context. Without context, this code is just a math problem.
November 10, 2009 at 5:45 pm
I agree with that comment. This was really written as a way to come up with an easy rule of thumb for combinations of three or more booleans. In this particular case, the logic (in sentence form) is something like:
“Redirect to another page if the user viewing this page is trying to view data from another user whose data they do not haver permission to see.”
So I use three checks:
Actually, reading that I realized that I introduced a bug when I worked on this earlier, and I need to go fix it now.