Tag Archives: best practices

The tyranny of poor tools (1)

I recently had the experience of checking in some changes to a source code file. I had changed just a few lines, but my editor (Emacs) had, as I had instructed it to do, automatically removed the trailing whitespace from each line whenever I executed the “save-buffer” command. Trailing spaces are worse than worthless, so removing them is a Good Thing, right?

Because the “diff” tool in common use where I worked was not configured to ignore differences consisting only of whitespace, each trailing space that Emacs had deleted was highlighted as a source code change demanding attention from the code reviewers. Instead of a few lines appearing as changed, there were dozens.

I found these spurious differences annoying, but thought the right thing for code reviewers to do was to ignore them, since these were not real changes.

Imagine my surprise when I was asked to put the deleted whitespace back!

The reason given was that, in addition to the burden on the code reviewer of having to ignore all these changes-that-are-not-really-changes, the source control software might have trouble handling those “differences”.

Now, the person who made the request is a smart fellow, no bozo. Yet he felt obliged to urge me to spend precious development time in order to better adapt to that broken source control software.

Of course, many organizations developing software have a filter that removes trailing whitespace as part of the check-in. But the relentless pressure to get things done, coupled with the fact that humans are more intelligent and more adaptable than either organizations or software, means that we humans do the adapting, even when the entire point of the organizational procedures or computer software is to help us do our jobs.

Copyright © 2009 Possum Technologies, All Rights Reserved.

Deeply nested statements

Problem: Deeply nested statements (usually “if” statements) tend toward unreadability.

Discussion: It is often best to rule out pathological cases before attempting to perform routine work. For example, one should ensure that a pointer is valid before attempting to dereference it, or that an array index is within the bounds of the array, or that one of many necessary functions has not returned a value indicating failure. Too many levels of nesting tend to obscure source code.

The structured programming dictum that a function must have exactly one entry and one exit point has led some to scrupulously avoid having more than one “return” statement in a function.

If all of the statements from some point onward in a function depend upon some condition and one is avoiding a short-circuiting “return” statement, one will naturally wrap the statements in an “if” test so as not to inappropriately execute statements. Thus the statements whose execution is conditional will inevitably get farther and farther from the test of that condition, impairing the readability of the source code.

    Hallmarks of excessive nesting:

  • There is a “then” part, but no “else”.
  • Every statement is wrapped in an “if” statement that does nothing but confirm that a particular value was or was not returned by the immediately preceding statement.

    Techniques to avoid excessive nesting:

     

  1. Instead of writing this:
    •    if (b1) {
         if (b2) {
            if (b3) {
               ...
               if (bn) {
                  S;
               }
            }
         }
       }
    • Do this:
      if (b1 && b2 && b3 && ... &&& bn) {
         S;
      }
  2. Instead of this:
    rc = f1 (x, y, z);
    if (rc == NO_PROBLEM) {
       rc = f2 (x, y, z);
       if (rc == NO_PROBLEM) {
          rc = f3 (x, y, z);
          if (rc == NO_PROBLEM) {
             ...
             rc = fn (x, y, z);
             if (rc == NO_PROBLEM) {
                S;
             }
          }
       }
    }
             

    Do this:

    rc = f1 (x, y, z);
    if (rc != NO_PROBLEM) {
       return;
    }

    rc = f2 (x, y, z);
    if (rc != NO_PROBLEM) {
       return;
    }

    rc = f3 (x, y, z);
    if (rc == NO_PROBLEM) {
       return;
    }

    ...

    rc = fn (x, y, z);
    if (rc != NO_PROBLEM) {
       return;
    }

    S;

    Better yet, if you are writing in a language like C++ or Ada that supports exceptions, have each of the called functions raise an exception on error instead of returning a value that must be checked by the caller (who may or may not do a necessary check). Then you can simply write the vastly more readable:

    // Each of the following statements may raise an exception:
    f1 (x, y, z);
    f2 (x, y, z);
    f3 (x, y, z);
    ...
    fn (x, y, z);

    S;

  3. Copyright  ©  2008 Possum Technologies, All Rights Reserved.

Avoiding unnecessary branching

Practice: Avoiding unnecessary branching.

Example (in C):
   if (c == 'N')
   {
       b = TRUE;
   }
   else
   {
       b = FALSE;
   }

Discussion: Since C inherited the conditional expression from Algol 60, we could write instead:
   b = (c == 'N') ? TRUE : FALSE;
But even that is over-complicated when we could—and should—write:
   b = c == 'N';
Besides being simpler, this avoids branching, resulting in smaller and faster generated code.

Objection: It is convenient to set breakpoints on the “then” and “else” alternatives of the “if” statement when debugging.

Answer: One can break on the assignment statement and examine its result.

Copyright © 2008 Possum Technologies, All Rights Reserved.

Comparing booleans

Practice: Comparing a boolean literal to a boolean result.

Example (in C): Let b be some expression having a boolean value (say, v1 > v2).
   if (b) ...

is better than

   if (b == TRUE) ...

and

   if (!b) ...

is better than

   if (b == FALSE) ....

Discussion: Booleans are sometimes referred to as “conditionals” in pre-ANSI C and have the type LOGICAL in Fortran.  The ANSI C standard guarantees that the result of a comparison always returns either TRUE (1) or FALSE (0). So were we to write

   if ((v1 > v2) == TRUE) ...
we would be comparing v1 and v2, which comparison would result in either TRUE or FALSE, which is fine, but then we would be further comparing that TRUE-or-FALSE result with the literal boolean value TRUE, which adds nothing useful to the more succint
   if (v1 > v2) ...

Objection: Some suggest that comparisons of boolean results with TRUE or FALSE is more readable.

Answer: Were
   ((v1 > v2) == TRUE)
more readable than
   (v1 > v2),
it follows that
   (((v1 > v2) == TRUE) == TRUE)
would be even more readable, and
   ((((v1 > v2) == TRUE) == TRUE) == TRUE)
more readable still, with further “improvements” in readability left as an exercise to the reader.

Copyright  ©  2008 Possum Technologies, All Rights Reserved.