Deep indentation

Having many levels of indentation can seriously hamper readability (and thus the understandability) of code. These indentation levels are normally a result of nested conditional statements and try-catch blocks.

How many levels are too many?

Many people feel that 4 levels of indentation is the limit. This is not a hard rule, but serves as a good guideline.

Another guideline is that your source file should never contain a line of code that gets wrapped around or needs side scrolling. Unfortunately this is dependant on your screen resolution and font size. For some people the limit is at 80 characters (the Linux kernel uses this measure) other people feel that this measure is outdated and wastes screen real estate.

The basic idea is that whenever the amount of indentation levels make it difficult to understand the code, it's too many.

How do I fix it?

One way suggested in “Refactoring: Improving the Design of Existing Code” by Martin Fowler is replacing the conditional with Guard Clauses. Ex.

method body
if condition1 is true
  if condition2 is true
     …
  else
     …

can be replaced with

method body
if condition1 is false
  return
if condition2 is true
  …
else
  …

Sometimes it is possible to rework the logic of the method. You may find that some conditionals are excessive or that it can be simplified further.

Many times though, this is an indication that other problems exist in your code. Things like methods that are too big, constant testing for null, using lots of “Type Codes”, implementing different states in one object, etc. In a lot of these cases, addressing the other problems will also fix the problem of multiple indentation levels.

Method stubs and Unused Code

A common thing we all do is creating stub methods on a class and implementing methods we do not immediately use. Both can lead to errors and incorrect results in parts of the code that use method stubs or (previously) unused code.

Method Stubs

Method stubs are functions that aren't implemented. The method body is normally empty, or returns a hard coded value in order to compile. Luckily they are easy to spot. There's nothing wrong in using method stubs when writing and testing the initial interface of a class, but no method stubs should be left hanging around in your code! If you do have method stubs, this can be corrected in one of two ways:

1. Remove the method stub

2. Implement the method stub

Most of the time you should remove it. You should only implement a method stub if it is actually used!

Unused Code

Another problem that is more difficult to spot is unused (and untested) code. If you are writing a library that are used by other people you cannot easily determine what methods are used and which ones are not. In this case your test cases should test all methods (actually all code paths).

The best way of avoiding unused methods, is to only create a method when you need it. This is stating the obvious, but we all tend to implement methods we don't use immediately (or ever). Lets look at an example:

A common class we all need and code at some stage is a Vector class. There are many well defined operations that can be performed on a vector, like calculating the length, converting it to a unit vector, addition, subtraction, dot product, cross product, etc. It is very easy to quickly go and define a vector and all the methods for working with it.

Chances are good that you do not immediately need all the methods that you thought up for you class and that that they will contain bugs. Once again, there are two ways of solving this:

1. Use the method (in a test case)

2. Remove the method

Option 1 should be executed if you still foresee that it is very likely that the method will be used in the near future or if the implementation entailed a large investment of effort. Most of the time you should strongly consider removing the method.

To sum it up: When writing code, be minimalistic, when using existing code, remove clutter.

Regards

Dirk