Code Advice #6: Write debuggable code!

WARNING: This blog entry was imported from my old blog on blogs.sun.com (which used different blogging software), so formatting and links may not be correct.

(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) {
bar();
}

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

getFoo().getBar().getBaz().doit();

or even the slightly less evil, but still bad

getFoo().doit();

What's wrong with this? Convenient Steppability^TM. 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();
foo.doit();

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...