03
Apr 06

Great article on refactoring…

Here‘s a great article on refactoring by Keith Ray.
Excellent.


03
Apr 06

Guard Clauses

Another interesting topic are Guard clauses as outlined in Michael Mahemoff’s Guard Clause Considered Helpful. Here’s my take on this topic:

How about rephrasing

void foo() {
 if (nothingToDo) { return; }
   // Implement typical behaviour of foo()
}

not like

void foo() {
  if (!nothingToDo()) { 
   // Implement typical behaviour of foo()
  }
}

but like

void foo() {
  if (someThingToDo()) { 
   // Implement typical behaviour of foo()
  }
}

The latter version avoids the double negative (thus concentrating on the standard case and not the exception), and I consider it quite readable in practice, e.g. if (resourceAvailable()) doSomeThing(). However, this will only be a viable alternative to the guard clause if foo() is very short. If foo() get’s so long that using the guard clause seems appropriate because the function will be more readable, maybe it’s time to take a long hard look at foo() to search for refactoring opportunities. Avoiding guard clauses may help in detecting functions which got out of control in terms of length & complexity – because there’s an awful lot of “intended code” in the function.

In the end, it boils down to personal preferences. Guard clauses are probably fine as long as the length & complexity of the function is monitored closely.


02
Apr 06

More on Single-exit functions

Ivan Moore posed the question what happens to the solution I suggested if the function returns the result of some other function:

void foo(boolean condition)
{
    int returnValue = calculateSomeValue();
    if (condition)
        returnValue = calculateSomeOtherValue();
    return returnValue;
}

Two considerations:

First, if calculateSomeValue() is O(1) or even O(n), I would still use the proposed single-exit/no-else variation. Truth to be told, I don’t even care if calculateSomeValue() is O(n*n) – I would go for the simple solution anyway. If, and only if, performance analysis shows that foo() is a system bottleneck, I would change it.

Second, if calculateSomeValue() has some undesired side-effects and thus should only be called if condition is indeed true. What’s “undesired side-effect” in this context? If the undesired side-effect is performance / resource-related (e.g. calculateSomeValue() goes out to fetch some data from a database or from a web-service), then it’s just a variation of the O(1)/O(n)/O(n*n) discussion above. I would still go for my single-exit approach and worry about performance later. On the other hand, if “undesired side-effect” means calculateSomeValue() does some changes to the systems state in addition to, well, calculating some value, then my suggestion is to check if this wouldn’t be great opportunity to improve the design & implementation of calculateSomeValue(). If that’s not possible, e.g. when working in a legacy-code environment, then I would go for the two-exits approach.