Saturday 9 February 2008

Refactoring / Design: Composed Method

I'm going to try and write a few posts over the next few weeks around the subject of Refactoring and Design. This is mainly practice for me, so that when asked to explain, I don't confuse the issue. All comments and suggestions are gratefully accepted.



When I am looking at code, I feel a lot more comfortable if a method is composed of several steps of similar context, in the appropriate order.

This is the Composed Method pattern from Smalltalk Best Practice Patterns.
In Refactoring To Patterns, Joshua Kerievsky says that this is one of the most important refactorings that he knows, and I would have to agree.

Here is an example that I just thought of (see if you can guess what I was doing):


public class CheeseCake {

private final Person chef;
private final Sink kitchen;
private CakeTin<CheeseCake> cake;

public CheeseCake (Person chef, Sink sink) {
this.chef = chef;
this.sink = sink;
}

public makeBase(Ingredients... ingredients) {
sink.runUnderWater(chef.getHands());
sink.useSoap(chef.getHands());
sink.dry(chef.getHands());
Bowl<Ingredients> bowl = new Bowl<Ingredients>();
for (Ingredients i : ingredients) {
bowl.add(i);
}
bowl.mix();
CakeTin<CheeseCake> cake = new CakeTin<CheeseCake>();
cake.add(bowl.contents());
}

}


Wow, we have quite a few smells in this code, and not the pleasant smells of cheesecake cooking either...
We'll ignore the Feature Envy and things for the moment, but I've only just written this code and already I'm having trouble telling what it does at a glance...

What about if the makeBase method looked like this:


public makeBase(Ingredients... ingredients) {
washHands();
Bowl<Ingredients> bowl = addTo(new Bowl<Ingredients>(), ingredients);
bowl.mix();
cake.add(bowl.contents(); // Moved CakeTin creation into constructor
}


That looks much better, but not only that, we also find that the washHands() method is used extensively. Now each step in the recipe doesn't need to implement their own hands washing routine.

It's also much more obvious that washHands has absolutely nothing to do with CheeseCake, and much more to do with either Person or Sink... maybe we'll look at this next time :-)