This was supposed to be a part of a much longer post on writing maintainable code, but I'm having trouble finishing the bigger post and I wanted to see an actual code-centric post before the new year.
I've talked a lot about Object Oriented concepts in the past, but there's always procedural code lurking inside each and every class. I feel very strongly that long methods and big classes are a veritable breeding ground for bugs. Small methods (and classes) are easier to troubleshoot by inspection, and hence, less likely to have bugs than a long method. To keep methods small and easily understandable, I like the Composed Method pattern first described by Kent Beck. The Composed Method pattern states that:
- Divide your code into methods that only perform a single, identifiable task.
- Keep all of the operations in a single method at the same level of abstraction. Think of a method that iterates through a collection and performs an operation with each child object. If we apply the Composed Method pattern we would move the operation on the child object into a separate method that is called from the main method.
- Following the Composed Method pattern will lead to the kind of small methods that make trouble shooting easier.
In other words, using the Composed Method means assiduously avoiding the ArrowHead Anti-Pattern (deep if/then or looping hierarchies in a single method).
Here's an example that I lifted from Joshua Kerievsky's Refactoring to Patterns book that illustrates the application of Composed Method. Pretending that the ArrayList class in .Net doesn't exist, create a resizable collection class that can be locked into a read only mode. The internal storage is just an array. As necessary the class will create a new, larger internal array. If we use pure brute force coding to build the Add() method in one big method we might get this code below.
public class MyExpandableList
{
private object[] _elements = new object[10];
private bool _readOnly;
private int _size = 0;
public void Add(object child)
{
if (!_readOnly)
{
int newSize = _elements.Length + 1;
if (newSize > _elements.Length)
{
object[] newElements = new object[_elements.Length + 10];
for (int i = 0; i < _size; i++)
{
newElements
= _elements;
}
_elements = newElements;
}
_elements[_size] = child;
_size++;
}
}
public bool ReadOnly
{
get { return _readOnly; }
set { _readOnly = value; }
}
}
It's not the biggest, hairiest method in the world, but it could definitely be better. Let's clean up the Add() method by doing these 6 refactorings.
- Invert the "readonly" check to a Guard Clause
- Extract Method - void addElement(object)
- Introduce Explaining Variable - shouldGrow
- Decompose Conditional - bool atCapacity()
- Inline Variable - shouldGrow --> atCapacity()
- Extract Method - void grow()
Applying that series of refactorings to the Add() method leads to this second version of the expandable list class.
public class MyExpandableListRefactored
{
private object[] _elements = new object[10];
private bool _readOnly;
private int _size = 0;
public void Add(object child)
{
if (_readOnly)
{
return;
}
if (atCapacity())
{
grow();
}
addElement(child);
}
private void grow()
{
object[] newElements = new object[_elements.Length + 10];
for (int i = 0; i < _size; i++)
{
newElements = _elements;
}
_elements = newElements;
}
private bool atCapacity()
{
int newSize = _elements.Length + 1;
return newSize > _elements.Length;
}
private void addElement(object child)
{
_elements[_size] = child;
_size++;
}
public bool ReadOnly
{
get { return _readOnly; }
set { _readOnly = value; }
}
}
I ended up adding several new methods, but the methods are very simple, and the Add() method is far more readable. I'm sure that the obvious objection to Composed Method is that the longer stack traces and call stacks making the code harder to debug. My simple answer is that a combination of using Test Driven Development and well factored code should minimize the need for the debugger.
Posted
Sun, Dec 3 2006 1:58 PM
by
Jeremy D. Miller