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!
