Monday, October 10, 2005

Code Advice #6: Write debuggable code!


(See intro for a background and caveats on these coding advice blog entries.)

It's important to write readable code. But that's not the whole story. Your code should be easy to debug too. In this blog entry, I'm pointing to some Bad code patterns that makes your code hard to debug.

The first, and most evil practice is to write a conditional on the same line as the if:

if (foo) bar();

Yes, this already violates other rules of good coding style in that it's missing braces. But even the cleaned up version is no better in terms of debugging:

if (foo) { bar(); }

This is Bad for debugging because as you're stepping through code, the whole if block becomes a single step! While you're debugging, you typically want to know if the conditional branch was taken or not. With the "correct" three line version this is not a problem; you step once over the if, and you can then separately decide if you want to step into the conditional method call for example.

if (foo) {

You may not realize you're guilty of the above problem, but if you use the ternary operator, a?b:c, you might be. The ternary operator is really an if-then-else block in disguise. Don't write code like this:

x = foo ? bar() : baz();

for the same reason as above: this is now a single debug statement so it's hard to decide if you want to step into or not (if you for example are only interested in stepping into if the target is baz().)

This frequently comes up in conjunction with return statements. I've seen coding styles recommend you to use ternary operators instead of if blocks. This is where code simplicity (for those who feel that a terse ternary statement is more readable) goes squarely against debuggability. I recommend you stick with the if block. I feel if blocks are more readable too. The argument for collapsing statements is usually that it makes the code more dense, so you can "fit more on the screen", but the same argument could be used against whitespace between logical code blocks and methods etc., and I think we all agree that optimizing for code density is not optimizing for code readability.

Another bad code pattern for debuggability is


or even the slightly less evil, but still bad


What's wrong with this? Convenient SteppabilityTM. In this pattern, all the method calls before the last one are simply getter calls. The doit() method is the interesting code we want to step into. However, since all the preceeding accessors are called in the same statement, we have to Step Into, then back out of, each method in the call chain up until we finally get to the interesting method!

Overshooting (the technique
of Stepping Over, then Undoing when you've gone too far
) doesn't work here.
There are some simple workarounds; for example, some debuggers let you put the caret on the last method, and then invoke a different stepping action to step into the desired method. But this is clearly inconvenient; you don't want to force users who are rapidly stepping with F7 and F8 to have to click to step into interesting code.

Thus, you should write the above code like this instead:

Foo foo = getFoo().getBar().getBaz();

Obviously, if any of getFoo(), getBar() or getBaz() were interesting too, you could break
this up further, but the whole point here is that you typically don't want to step into accessors (which I'm assuming the above are) so get those into their own statement. Now you can step right over the first line and into the second.

Tools can help this situation. In Sun's C++ development environment, there is a fourth Stepping action: Run Into Last Statement Call. This stepping action "magically" achieves precisely what you want: It analyzes all the method calls in the current statement, and steps into the last such statement. The net result in that the code example above (before we broke it up) you could click on the Run Into Last Statement Call button, and it would step right into doit() ! I really really want that action in the NetBeans debugger. I guess it will require some fancy bytecode analysis. The screenshot on the right shows the Stepping action part of the menu in Sun Studio - the last action is the magical one...


  1. Hi Tor,
    What do you think about the following with regard to code comments and debugging?
    If you use the /**/ style comment inside a method
    it makes it awkward to comment out the entire method and stub it when trying to track down a bug or test and alternate implementation.
    If you only use // style comments inside methods its easy to use /**/ comments to comment out the whole method.

  2. I disagree with you post this time.
    I find it a worse evil to make the code longer and more unreadable, just because whatever tool you are using only does step-by-step on a line basis.
    I once worked with Visual age for java 4 (pretty old ,I know). It did "true" statement by statement stepping.
    With that foo and bar() from your example 1 would be two different steps, and I could choose step in or step over for each of them.
    It also worked with your long getX().getY().doIt() calls and ternary operator. Each part of statement will be a step for itself.
    I would rather have the tools improve than altering code.

  3. If only I could get my coworkers to believe this, my life would be easier. :/

  4. Coding if and ternary operators on a single line is very helpful if the conditions are rarely taken. I often use this style when I care more about exposing multiple items I'm testing and the actions taken. Carefully coded, the if expressions jump out, so you can quickly scan a list of tests and explore the one you're interested in.
    Humans are good at pattern recognition ... terrible at things that involve memory. By extending the code vertically, the pattern disappears and humans must now "remember" the code and read all of it to understand.
    I don't code all if's that way, but sometimes it just makes more sense so you can quickly grasp each of the cases tested and executed.
    I guess I spend more time reading code than stepping thru it with a debugger. Columns of tests and actions are easier to grasp (by pattern).

  5. Um... Let's see... I guess this means one should absolutely never use a *shudder* macro.
    Shaun -- /* */ is a perfectly fine comment style as is pretty much any other comment style. The correct way to comment out multiple lines of C[++] code is via "#if 0 ... #endif". [Your favorite language doesn't support that comment style? Bummer, you should get better tools.]