Wednesday, October 12, 2005
Code such as the following leaves readers scratching their heads:
if IsNotUninhibited() != true then DoSomething() else DoSomethingElse() end
Under what circumstances does DoSomething() get called? Well, it obviously has something to do with whether something is "inhibited" or not, but with all those negations mixed in, it's hard to tell if DoSomething() gets executed when inhibited or when not inhibited.
When I find something like this, I try to refactor it into something easier to understand by eliminating the excess negations. Here are some of the principles I follow:
- Use identifiers that don't include any negations. This basically means that the identifiers should not include the prefixes "not-," "non-," "un-," "in-," and so on. So instead of "IsNotUninhibited," use the equivalent but simpler name "IsInhibited."
- Use the positive form of names instead of the the negative form. For example, "inhibited" really means "not enabled," so instead of "IsInhibited," the code would be easier to understand if the name was "IsEnabled." (Of course, this reverses the meaning, so if you change the name, be sure to negate or un-negate the uses appropriately.)
- Never compare a boolean expression to the value true or false (or whatever the analogues are in whatever programming language is in use). Not only is the expression simpler and therefore easier to read without the comparison, it also eliminates common mistakes in those programming languages where all non-zero values are considered true, or where multiple values can be treated as false. So instead of "if X == true then ...," just write "if X then ...," and instead of "if X == false then ...," write "if not X then ...." Note that in some popular programming languages, the word "not" is spelled as "!," but everybody knowns that it is still pronounced "not."
- When writing an "if ... then ... else," try to avoid putting any "nots" in the initial condition. So the part after the if will be the "true" path, and the part after the else will be the "false" path.
So if we follow these principles, the above code could be rewritten as
if IsEnabled() then DoSomething() else DoSomethingElse() end
Would you fail to deny the rewrite isn't not a lot less unclear?