Send As SMS

Bent Andre Solheim Blog

Friday, November 10, 2006

Guard Clauses and Composed Methods

As I have told you before, I have worked with several legacy systems so far in my career. Most of these systems have had code that was hard to read and understand. When working with the code to increase readability, I find myself doing the same two refactorings over and over again. In fact, I find myself doing these refactorings pretty much all the time when writing new code aswell.

The first one is Replace Nested Conditionals with Guard Clauses. Nested Conditionals, also called the the Arrow Anti-pattern, is when a method has conditional logic that makes it unclear what the normal path of execution is. Let me give you an example;

Result res;
if (user.privileges.get("admin") != null) {
if (connectionPool != null) {
if (connectionPool.hasAvailableConnections()) {
if (webService != null) {
res = doSomethingWithDbAndWs(
connectionPool.connection(), webService);
} else {
res = null;
}
} else {
res = null;
}
} else {
res = null;
}
} else {
res = null;
}
return res;

Although the essence of this method is not complex, the nested conditionals make it very hard to get a feeling of the logic. Refactoring to use guard clauses makes it much easier to read;

if (user.privileges.get("admin") == null) return null;
if (connectionPool == null) return null;
if (!connectionPool.hasAvailableConnections()) return null;
if (webService == null) return null;

return doSomethingWithDbAndWs(connectionPool.connection(), webService);

Returning from the method as quickly as you possibly can will often improve readability and reduce its cyclomatic complexity. Cyclomatic complexity is a measure of how many distinct paths there are through code, and studies show a correlation between a program's cyclomatic complexity and its error frequency. You should use Guard Clauses whenever you can.

The second refactoring I tend to come back to is Compose Method. Compose Method is much harder to explain than Guard Clauses, because there are no defined steps to perform to complete the refactoring. In his book "Refactoring to Patterns", Joshua Kerievsky explains it as a "[transformation of] the logic into a small number of intention-revealing steps at the same level of detail". This will result in programs with many small methods, each a few lines long. Let me give you an example (I stole this from Refactoring to Patterns);

public void add(Object element) {
if (!readOnly) {
int newSize = size + 1;
if (newSize > elements.length) {
Object[] newElements = new Object[elements.length + 10];
for (int i=0; i<size; i++) {
newElements[i] = elements[i];
}
elements = newElements;
}
elements[size++] = element;
}
}

I cannot rapidly see what this code does. I have to read through it thoroughly to understand the logic. Performing the Extract Method refactoring a couple of times makes for a whole different experience:

public void add(Object element) {
if (readOnly)
return;
if (atCapacity()) {
grow();
}
addElement(element);
}

There are several reasons why the refactored version is earsier to read. It is much shorter, and it is now composed of method calls to well named methods that describe what operations are performed. In addition we converted the nested conditionals to guard clauses.

If you consistently apply Composed Method, you will find that most of your code comments will be superfluous; your methods will be self documenting. In the long run, self documenting composed methods are easy to maintain.

I find Guard Clauses and Composed Methods to be two extremely important techniques for writing maintainable code. If you do not have any experience with either, I would recommend you take steps to learn them both. It will pay off, guaranteed!

3 Comments:

  • Great post, Bent! I've been thinking about posting something about these too but never got around to it. I too think these are the most important refactorings. They make the code very clear and readable. I tell people that if they need to be adding comments to explain the flow of their logic, they should be making composed methods instead. Guard clauses are also great for writing unit tests. Edge-case tests and the guards go hand in hand. (BTW: Your original add() method will add the element even if readOnly is true.)

    By Bjørn Erik Haug, at 7:52 PM  

  • Thank you for your nice comments, Bjørn Erik. It is nice to see that people out there check out my blog from time to time! :)

    Thank you for the tip on the error in the add-method. I have fixed it now.

    Also thank you for the follow-up posting on your site. I appreciate it!

    By Bent André Solheim, at 12:36 AM  

  • Great refactoring tips! The Compose Methods one is nothing short of being pseudo-codish in nature. Only problem being that it would involve actually writing pseudo-code before hastily jumping in to hack away at the keyboard :)

    By Joel, at 9:16 PM  

Post a Comment

<< Home