Composed Method Pattern

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:

  1. Divide your code into methods that only perform a single, identifiable task. 
  2. 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.
  3. 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[i] = _elements[i];

                    }

 

                    _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.

  1. Invert the "readonly" check to a Guard Clause
  2. Extract Method – void addElement(object)
  3. Introduce Explaining Variable – shouldGrow
  4. Decompose Conditional – bool atCapacity()
  5. Inline Variable – shouldGrow –> atCapacity()
  6. 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[i] = _elements[i];

            }

 

            _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.

About Jeremy Miller

Jeremy is the Chief Software Architect at Dovetail Software, the coolest ISV in Austin. Jeremy began his IT career writing "Shadow IT" applications to automate his engineering documentation, then wandered into software development because it looked like more fun. Jeremy is the author of the open source StructureMap tool for Dependency Injection with .Net, StoryTeller for supercharged acceptance testing in .Net, and one of the principal developers behind FubuMVC. Jeremy's thoughts on all things software can be found at The Shade Tree Developer at http://codebetter.com/jeremymiller.
This entry was posted in Design Patterns, Maintainability. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://www.devhints.blogspot.com thiagodp

    @Justin Michel

    I don’t like the idea of growBy modifing _element and _size and addElement have to know about it. Instead of knowing about the internals of growBy and make:

    growBy( 1 );
    _element[ _size - 1 ] = o;

    it’s better create a setLastElement method that does it:

    public void setLastElement(object o)
    {
    _element[ _size - 1 ] = o;
    }

    So we will have:

    public void addElement(object o) {

    if (_readOnly) {
    throw new ReadOnlyException("Can't add to readonly list.");
    }

    growBy( 1 );
    setLastElement( o );
    }

  • http://colinjack.blogspot.com/ Colin Jack

    Good article, as stated by a previous reply though I don’t like the readonly guard statement silently returning.

  • http://www.bellware.net ScottBellware

    SteveJ,

    I do it with tests. By convention, I like to keep tests that demonstrate the primary responsibility(s) of a class at the top of the test class. Tests that exercise helper methods show up later in the class.

    I don’t really grok the idea of a component having an entry point though. That’s not really part of my design work. If a component is a container for classes that are focused on the same architectural concern, how would one class in the component by more primary than another?

    IMO, the best way to demonstrate what a class is for is to just demonstrate it with client code. The best way to do that in my experience is to use a client-driven design practice like test-driven development, behavior-driven design, etc.

  • SteveJ

    I’m a big fan of small classes and small methods, but I find a great deal of people have a hard time knowing where to start when they’re trying to figure a component out and it doesn’t have one humongous master class that does everything important. Has anyone come across a good way to provide a centralized starting point for a major component without resorting to code bloat? I guess I’m looking for something more substantial than a common interface pattern, the people I work with at least want to dig down into the source and see what the code is REALLY doing before they call methods on it (i.e abstraction isn’t really trusted).

    For instance I’m writing a pretty complex component that has 150+ classes (50,000+ LOC). (I’ve seen linux kernels that try to do less than this component). Even those classes are a bit too big for me. But I’m wary of refactoring farther when I know it’s already too hard for someone else to figure out what all those packages are for.

    Anyway, any concrete suggestions and examples would be useful. Are there any open source projects that you know of that seem pretty easy to grasp even if they are matching the complexity of an OS? I’d like to have a structure (outside of a design doc or comments) that allows a maintenance programmer the ability to grasp what each package does at a glimpse. Something self-referential so one could work their way backwards and forwards through the code without a rosetta stone.

  • Steve Mitcham

    I spotted the at capacity bug as well. What I’ve noticed as I’ve gotten into refactoring and this composed method pattern is that I’ll pick apart some code into separate methods. The new methods can usually be optimized now that they are separate and .

    If the new smaller (many times one line) method is not being used in multiple places, I can put it back in the original method from which it was extracted. When I’m finished, a 30 line method has been reduced to a 10 line method with much more readable structure.

  • http://ihorwill.blogspot.com Ian Horwill

    You’re right – refactoring does help you spot bugs! atCapacity() always returns true.

  • http://justin-michel.spaces.live.com/ Justin Michel

    1) The Add() method silently does nothing if the list is in ReadOnly mode. If you think about it this is really conceptually equivalent to writing the following:
    public void Add(object 0) {
    try {
    if (_readOnly) {
    throw new ReadOnlyException(“Can’t Add to ReadOnly List.”);
    }

    } catch (ReadOnlyException) {
    // ignored
    }
    }

    It would be better to create a ReadOnlyException class and throw from Add().

    2) The atCapacity() method always returns true. It should be comparing _size to _elements.Length.
    I also don’t see any value in this trivial method. Is the following really any less readable?
    if (_size >= _elements.Length) {
    growBy(1);
    }
    It also might be a little easier to read IMHO if growBy() contained the check.

    3) This code shouldn’t use a for loop to copy array elements. Array.Copy() already exists.

    4) As with atCapacity(), the addElement() method is really too trivial to warrant its own method.

    5) Growing by 10 seems rather strange. The generally accepted practice is to grow by 50%.

    I might do it like this:

    public class MyExpandableList {
    private int _size = 0;
    private object[] _elements = new object[INITIAL_SIZE];
    private bool _readOnly;
    public void Add(object o) {
    if (_readOnly) {
    throw new ReadOnlyException(“Can’t add to readonly list.”);
    }
    growBy(1);
    _elements[_size - 1] = o;
    }
    private void growBy(int count) {
    _size += count;
    if (_size < _elements.Length) {
    return;
    }
    object[] tmp = new object[_size + (_size / 2)];
    Array.Copy(_elements, 0, tmp, 0, _elements.Length);
    _elements = tmp;
    }
    public bool ReadOnly {
    get {
    return _readOnly;
    }
    set {
    _readOnly = value;
    }
    }
    }