From a project I was working on recently I ran into this problematic and fairly complex boolean expression (pseudocode):
if not arguments.pending and arguments.umoneyamount is 0 and aname.agencyname is arguments.agencyname
do something
elseif ((aname.agencyname is arguments.agencyname and arguments.pending) or (session.id is 1 or (isdefined("session.realid") and session.realid gt 0)) or (not arguments.pending and arguments.umoneyamount is 0 and local.isMyClient)) and (session.id is 1 or arguments.pending)
do something else
else
do the other thing
end
Or alternatively,
local.isMyClient = aname.agencyname is arguments.agencyname
local.isPending = arguments.pending
local.isAdmin = session.id is 1 or (isdefined("session.realid") and session.realid gt 0)
local.canAddUtilities = not arguments.pending and arguments.umoneyamount is 0 and local.isMyClient
local.allow_edit = (local.isMyClient and local.isPending) or local.isAdmin or local.canAddUtilities
if local.canAddUtilities
do something
elseif local.allow_edit and (session.id is 1 or arguments.pending)
do something else
else
do the other thing
end
Which do you prefer, and why?
Hey! Why don't you make your life easier and subscribe to the full post
or short blurb RSS feed? I'm so confident you'll love my smelly pasta plate
wisdom that I'm offering a no-strings-attached, lifetime money back guarantee!
Leave a comment
The latter is better (assuming they do the same thing) and here is why - it breaks separates understanding the logic from understanding the implementation. In the first block, everything is jumbled together so you have to pick it all apart before you can even really see what the point of the CFIF statement is. In the second block, the small "if clauses" are more meaninful and easy to read, and therefore you can concentrate on the outcome / goal of the CFIF statement and not worry about how the states are caclucalted... my two cents.
Posted by
Ben Nadel
on Aug 01, 2007 at 04:08 PM UTC - 6 hrs
In fact, I would go further and separate out the complex conditionals into their own private methods. This makes the original method more cohesive (it isn't cluttered with all those decision resolution statements), and lets you reuse the logic in other methods in the same component (which is a very common thing).
Posted by
Brian Kotek
on Aug 01, 2007 at 09:11 PM UTC - 6 hrs
Second one, provided they do the same thing.
Reason is it modularises the condition, if something goes wrong and I had to inspect the conditions it would be easier the second way because it's basically taking something complicated and breaking it down into steps which are easier to inspect/understand.
cheers,
Sid.
Posted by
Sid
on Aug 02, 2007 at 04:11 AM UTC - 6 hrs
Funnily enough, I was reading Martin Fowler's refactoring conditional logic chapters from his refactoring book last night. Some of the solutions he offers pretty much mirror what Brian has suggested.
Posted by
James Netherton
on Aug 02, 2007 at 08:00 AM UTC - 6 hrs
The latter. Why? Readability. The second allows me to look at two simple evaluation groups rather than one hairball. I can get grasp the meat of what you're doing much faster that way if I have to pick up your code in a year or so. :-)
Posted by
Rob Wilkerson
on Aug 02, 2007 at 01:29 PM UTC - 6 hrs
Thanks everyone for all the comments. I responded with a small post about the reason for the question:
http://www.codeodor.com/index.cfm/2007/8/5/Re-Comp...If you don't want to click, the meat of it:
I just wanted to highlight that you can use variables to store results of complex boolean expressions. Of course, most people already know this. Even I know it.
So why is it when I look through code I see so little use of it?
Posted by
Sam
on Aug 05, 2007 at 10:20 PM UTC - 6 hrs
