Greg Beech's Website

Not using negatives does not make your code less readable

Most people find negative statements more difficult to comprehend than positive statements. You probably had to double-check the title of this entry to see if it made sense - I know I did! This isn't surprising if you think about it because a positive statements presents a concept which can be used directly, whereas a negative statement presents a concept and then says "the opposite of that" so you need to work out what the opposite is and remember that instead.

An example I came across this week in our codebase was a method to decide whether a user of the site needs to accept the site terms before they can view a given video. The rule is that if a video needs the terms to be accepted, the user must either be authenticated (as they agreed to the terms on joining) or have a cookie stored indicating they have accepted the terms. The original code was something like the following:

bool mustAcceptTerms = false;
if (!Thread.CurrentPrincipal.Identity.IsAuthenticated &&
    !UserHasAcceptedTerms() &&
    !CanViewVideoWithoutAcceptingTerms())
{
    mustAcceptTerms = true;
}

Looking at this code, in which all statements are negated, I can't quickly reason whether it actually meets the rule. If we re-state the rule as the logically equivalent "if the user is not authenticated, and the user has not accepted the terms, then if the video cannot be viewed without accepting the terms, the user must accept the terms" we can see that it in fact does, but is that the sort of specification you want to code against?

Me neither.

By using primarily positive statements and breaking terms that need negation out into separate statements we can rewrite the code to match the semantics of the original rule, making it trivial to reason that it is correct. I also reversed the name and semantics of the helper method CanViewVideoWithoutAcceptingTerms to MustAcceptTermsToViewVideo as that matches what we really want to know.

bool hasAcceptedTerms = 
    Thread.CurrentPrincipal.Identity.IsAuthenticated ||
    UserHasAcceptedTerms();
bool mustAcceptTerms = 
    !hasAcceptedTerms && MustAcceptTermsToViewVideo();

If you have more than a single negation in a decision block such as an if or while, then consider refactoring the code. You'll be more likely to get it correct, and people who have to maintain it in the future will thank you (or at least won't curse you).


Posted Feb 16 2008, 04:14 PM by Greg Beech
Filed under: ,

Add a Comment

(required)  
(optional)
(required)  
Remember Me?

Enter the numbers above:
Copyright (C) Greg Beech. All rights reserved.
Powered by Community Server (Non-Commercial Edition), by Telligent Systems