Refactoring My Template Method Post

Evan comments on my post I Heart Template Method:

one tiny suggestion for this object, instead of building the “one object to rule them all” base class, you could do the following:

create an interface IExecutable with one method, Execute()

then build a class for each technical concern (logging, transactions, security, eventing aka observation, caching)…

each of the classes can implement IExecutable and take another IExecutable in their constructor… it’s a simple way to layer each on top while keeping true to the single responsibility and open/closed principles..each class performs its responsibility and then delegates to the next object (passed into the constructor) until the final object gets executed (the real Task object).. it gives you the freedom to change one technical concern without affecting the others..or providing multiple implementations of a particular concern (ie..two caching modes). you can also apply some concerns to some tasks while leaving others off (or rearranging the order of execution).. you can think of them as mini-layers on top of your task object (which also implements IExecutable)

you can either wire the whole thing up with a DI container or create a factory to add the objects onto the task objects (depending on the app)

Great comment. I’m not sure this is a one object to rule them all situation, but maybe so I’ll bite and get to refactorin’. Recall my original, now bug free, example:

public abstract class TaskBase
{
   public void Execute()
   {
      using (TransactionScope scope = new TransactionScope())
      {
         OnBeforeExecute();

         ExecuteTask();

         OnAfterExecute();
         LogExecution();
         scope.Complete();
      }
   }

   protected void OnBeforeExecute()
   {
      if (BeforeExecute != null) BeforeExecute(this, EventArgs.Empty);
   }

   protected void OnAfterExecute()
   {
      if (AfterExecute != null) AfterExecute(this, EventArgs.Empty);
   }

   public event EventHandler BeforeExecute;

   public event EventHandler AfterExecute;

   private void LogExecution()
   {
      // write event details to 
      // some durable resource
   }

   protected abstract void ExecuteTask();
} 

Template Method by Way of Generics, Constructor Injection

The first piece of the comment I’ll tackle is the idea that I have this big, abstract, “magic”, do-it-all class and that my simple framework based on Template Method is violating Single Responsibility Principle (SRP) and Open-Closed Principal (OCP). I’ll buy that. I can decouple the base class from our individual task implementations by keeping the Template Method but moving away from a Layer SuperType. I’ll define an ITask class and a TaskExecutor<TTask> class. We can constrain TTask to require a class implementing ITask. 

Furthermore, I’ll make TaskExecutor<TTask> use constructor injection so my transaction service can be swapped out in regular code, an IoC container, or with a mocking framework. This step gets me back in the good graces of SRP. It’s worth mentioning that I’m using a tiny Role Interface for ITask. My task classes can now be straight-up POCOs with all the associated benefits. I’m still keeping the spirit of Template Method here, just freeing up my one base class for whatever class ultimately inherits ITask. It’s interesting to note that in the C++ world the (more-or-less) equivalent of C# Generics are called Templates.

The code after these changes:

public interface ITask
{
   void Execute();

   bool NeedsTransaction { get; }
}
public class TaskExecutor<TTask>
   where TTask : ITask
{
   private readonly TTask _task;
   private readonly ITransactionService _transactionService;

   public TaskExecutor(TTask task)
   {
      _task = task;
      if (_task.NeedsTransaction)
         _transactionService = new ProductionTransactionService();
   }

   public TaskExecutor(TTask task, ITransactionService service)
   {
      _task = task;
      _transactionService = service;
   }
   public void Execute()
   {
      if (_task.NeedsTransaction)
         ExecuteTransactionalTask(_task);
      else
         _task.Execute();
   }

   private void ExecuteTransactionalTask(TTask task)
   {
      using (ITransactionScope scope = _transactionService.GetScope())
      {
         task.Execute();
         scope.Complete();
      }
   }
}

Did you notice I have two constructors? The constructor with two arguments is used for tests. I’d also use it if it every became desirable to introduce an Inversion-of-Control container in my application. I’ve created an ITransactionService interface that’s easily faked by hand or mocked using your favorite mock object framework. Of course my original post was way too ambiguous. It assumes a framework where all tasks are transactional. Shame on me for not being explicit with this, I can see reason to cry foul. Let’s right previous wrongs and make things explicit.

Our ITask implementations must tell our framework if it needs a transaction by giving it a NeedsTransaction property. We could choose to take control of transactions inside each individual ITask implementation, but that kind of muddies the goal of making ITasks dead easy and focused on the, well, task at hand. Furthermore, it violates another principal of good design: Don’t Repeat Yourself, The DRY Principal. Balancing all of these concerns I’m making the decision to leave transaction coordination inside the TaskExecutor. This still leaves us with a good SRP quotient; TaskExecutor now depends on a seperate service for transactions. 

OK, enough on that.

Extending the Behavior of Task Execution

Here we have a couple of options. I’ve mentioned Chain of Responsibility, so let’s go down that route.I’ll define a handler abstract base class for code in our pre- and post-execution pipeline. With my alternative design I’ll swap out Observer (the BeforeExecute and AfterExecute events) for two properties and some additional logic in TaskExecutor.Execute(). Now, an important question I have to ask myself here: does the order of pre- and post-execution handlers matter? For the sake of argument, let’s say “sure”, Chain of Responsibility is a good choice, and move on with life.

Here’s what the refactored TaskExecutor class looks like with the changes in bold:

public class TaskExecutor<TTask>
   where TTask : ITask
{
   private readonly TTask _task;
   private readonly ITransactionService _transactionService;
   private ExecutionChainHandler<TTask> _beforeExecutionChain;
   private ExecutionChainHandler<TTask> _afterExecutionChain;

   [CoverageExclude]
   public TaskExecutor(TTask task)
   {
      _task = task;
      if (_task.NeedsTransaction)
         _transactionService = new ProductionTransactionService();
   }

   public TaskExecutor(TTask task, ITransactionService service)
   {
      _task = task;
      _transactionService = service;
   }

   public ExecutionChainHandler<TTask> BeforeExecutionChain
   {
      set { this._beforeExecutionChain = value; }
   }

   public ExecutionChainHandler<TTask> AfterExecutionChain
   {
      set { this._afterExecutionChain = value; }
   }

   public void Execute()
   {

      if (this._beforeExecutionChain != null)
         this._beforeExecutionChain.HandleWithinChain(_task);

      if (_task.NeedsTransaction)
         ExecuteTransactionalTask(_task);
      else
         _task.Execute();

      if (this._afterExecutionChain != null)
         this._afterExecutionChain.HandleWithinChain(_task);

   }

   private void ExecuteTransactionalTask(TTask task)
   {
      using (ITransactionScope scope = _transactionService.GetScope())
      {
         task.Execute();
         scope.Complete();
      }
   }
}

I’ve also defined another class ExecutionChainHandler and its implementation looks like this:

public abstract class ExecutionChainHandler<TTask> where TTask : ITask
{
   private ExecutionChainHandler<TTask> _nextHandler;

   public ExecutionChainHandler<TTask> NextHandler
   {
      set { _nextHandler = value; }
   }

   public void HandleWithinChain(TTask task)
   {
      Handle(task);
      if (_nextHandler != null)
         _nextHandler.HandleWithinChain(task);
   }

   public abstract void Handle(TTask task);
}

By defining both pre- and post-execution Chains of Responsibility I stay on the good side of the Open-Closed Principle. It’s certainly easy to modify Task execution and inject additional validation or processing logic before and after.

One thing you’ll notice in this framework is that ExecutionChainHandler<TTask> takes a specific task. This approach wouldn’t be ideally suited for creating generic services like caching… at first glance. We can, however, treat each implementation of ExecuteChainHandler<TTask> as a Adapter into a common, generic service. Say we have a task implementation called GetCustomerData. Maybe in the pre-execution chain we want to check to see if some necessary data is in a cache. In this case, with what I’ve got here, I’d probably need to refactor a “shortcut” or “interrupt” mechanism to enable ExecutionChainHandler’s to shortcut the rest of the chain and task execution as a whole (perhaps), but I’d certainly employ an Adapter to a more cross-cutting, but single responsibility, service. One can imagine a generic ICacheService.

What’s the Point?

Is this code complete? Hell no. Does it represent anything real world? Well I suppose it could, but that’s not my intention.

On the surface I’m illustrating a better, more testable, generics-based Template Method. I show how you can select and apply lots of patterns in the process of defining a little framework. The keyword there is selection. When you first get into this stuff there’s an awful tendency to indulge in using every pattern you can think of: solutions in search of a problem. Patterns can be your friends and work well together when you start out with a simple approach and match them to the problem your are facing. The key is to break down the problem into small chunks then, when it makes sense, apply the pattern that matches a) the context of your problem and b) whose forces you can live with.

Beyond solving business problems, you need to consider the problem of the maintainability of your code. Tried-and-true principles such as OCP and SRP belong in every developer’s default checklist and go a long way in deciding which patterns to employ and where.

Comments (2) left to “Refactoring My Template Method Post”

  1. Evan wrote:

    kudos on the refactoring. i promise to post some code this week for you to take a poke at.. :-)

  2. Dave wrote:

    Code battle!

Post a Comment

*Required
*Required (Never published)