Big Refactoring: Separate Domain from Presentation

In his landmark book Refactoring: Improving the Design of Existing Code, Marting Fowler not only presents a catalog of “regular” refactorings, he also mentions some “big” refactorings. These big refactorings are not described as a series of atomic steps to follow, but more as a recipe for using a longer series of regular refactorings. And since they are bigger than regular refactorings, they also take a lot longer to complete, sometimes even months.

I’m in the middle of one of these: Separate Domain from Presentation. Now, we all know that we shouldn’t put business logic in interface code, so why do I find myself in this situation?

Well, technically, I don’t 😉 We use Struts, which has a nice MVC architecture. However, it’s the Controller part that has me worried. In Struts, one writes Action classes to control application flow:

“The goal of an Action class is to process a request, via its execute() method, and return an ActionForward object that identifies where control should be forwarded (e.g. a JSP, Tile definition, Velocity template, or another Action) to provide the appropriate response.”

It is, however, all too convenient to implement the business logic in Action classes as well. The Struts documentation even warns about this danger:

“Perform the processing required to deal with this request (such as saving a row into a database). This can be done by logic code embedded within the Action class itself, but should generally be performed by calling an appropriate method of a business logic bean.”

And that’s exactly what’s happened in our code. So I guess I’m actually in the middle of Separate Domain from Controller 😉

Fixing this is not a trivial task. The Action classes use ActionForm classes that hold data entered in the UI to perform their work. This ties them to Struts, which I don’t like at all. For instance, it makes it very hard for us to switch to a different web framework, should we so choose. It also means that simple solutions like Extract Method won’t work, since the extracted method would get the ActionForm as a parameter.

My solution has been to introduce what I call Service classes. A Service class has one method that implements the service that the Action provides. The method has one parameter, which is a Parameter Object, that contains the same information as the Action‘s ActionForm does. I call them Service classes, since these classes could very well be used to implement web services as well.

Anyway, all the Action class has to do now, is:

  1. instantiate the appropriate Parameter Object
  2. populate it from the ActionForm
  3. instantiate the appropriate Service class
  4. call the appropriate method on the Service class (passing the Parameter Object)
  5. update the ActionForm from the method’s result object
  6. construct an ActionForward (possibly using information from the result object)

Luckily, I could automate all of that in a base class using reflection, so that each Action class now only needs two methods: one for instantiating the Parameter Object, and one for instantiating the Service class.

Still, that leaves a lot of Actions to convert. And to make matters worse, they are organized into class hierarchies, which makes it hard to convert them one by one. So I guess I won’t be sitting idle any time soon…

The Law of Demeter

In my previous post I used the Law of Demeter as a motivation for the Hide Delegate refactoring:

The Law of Demeter for functions requires that a method M of an object O may only invoke the methods of the following kinds of objects:

  1. O itself
  2. M’s parameters
  3. any objects created/instantiated within M
  4. O’s direct component objects

Code that violates the Law of Demeter is a candidate for Hide Delegate, e.g. manager = john.getDepartment().getManager() can be refactored to manager = john.getManager(), where the Employee class gets a new getManager() method.

However, not all such refactorings make as much sense. Consider, for example, someone who’s trying to kiss up to his boss: sendFlowers(john.getManager().getSpouse()). Applying Hide Delegate here would yield a getManagersSpouse() method in Employee. Yuck.

I have a couple of problems this use of Hide Delegate. First of all, it creates methods that by definition reek of feature envy. Second, methods like getManagersSpouse() clearly violate the single responsibility principle. Finally, the Law of Demeter clashes with the concept of fluent interfaces.

Luckily, you can always back out from an adverse Hide Delegate by applying the opposite refactoring: Remove Middle Man.

Automating refactorings

I’m a big fan of both refactoring and automation. It’s no wonder, then, that the support for automated refactoring in Eclipse makes me very happy. I find that it makes me a lot more productive, and I produce better code. That’s because performing a refactoring is easy and fast enough to actually do it.

I also find that I refactor routinely. Where Martin Fowler, in his classic book Refactoring, gives the advice not to mix refactoring and adding new functionality, I do it almost mindlessly anyway. No need to run unit tests before and after the refactorings, since I know they Just Work™.

Not so with any refactorings that are not supported by the tool, though. For instance, when trying to adhere to the Law of Demeter, one would want to perform the refactoring Hide Delegate. Unfortunately, Eclipse has no support for this refactoring 😦 You can, however, simulate this refactoring using a combination of other refactorings. Let me explain that using a simple example.

We start with the following abstract code that shows the situation before we want to apply Hide Delegate:

public class Client {

  public void example() {
    final Server server = new Server();
    server.getDelegate().method();  
  }

}

public class Server {

  private final Delegate delegate = new Delegate();

  public Delegate getDelegate() {
    return delegate;
  }
  
}

public class Delegate {

  public void method() {
   // Do it...
  }
  
}

First, we perform Extract Method on server.getDelegate().method() (make the method public):

public class Client {

  public void example() {
    final Server server = new Server();
    method(server);  
  }

  public void method(final Server server) {
    server.getDelegate().method();
  }

}

Next, perform Move Method to move method() to Server:

public class Client {

  public void example() {
    final Server server = new Server();
    server.method();  
  }

}

public class Server {

 private final Delegate delegate = new Delegate();

  public Delegate getDelegate() {
    return delegate;
  }

  public void method() {
    getDelegate().method();
  }
  
}

And, voila, we have performed Hide Delegate!