Slicing Concerns: Implementations

In Slicing Concerns And Naming Them I posed a question about how to go about separating different concerns while still maintaining a clean and relatable code base.  Some interesting conversation resulted, and I wanted to follow up by investigating some of the different approaches to this problem that I'm aware of.

Inheritance

public class Task : ActiveRecord
{
  public string Name { get; set; }
  public int AssignedTo_UserId { get; set; }
  public DateTime DueOn { get; set; }
}

public class NotificationTask : Task
{
  public override void Save()
  {
    bool isNew = IsNewRecord;
    base.Save();
    if (isNew)
      Email.Send(...);
  }
}

public class TasksController : Controller
{
  public ActionResult Create(...)
  {
    ...
    new NotificationTask {...}.Save();
    ...
  }

  public ActionResult CreateWithNoEmail(...)
  {
    ...
    new Task {...}.Save();
    ...
  }
}

This works, and the names are reasonable. But of course, inheritance can cause problems... I wont go into the composition over inheritance arguments as I assume this isn't the first time you've heard it!

Decorator

public class Task : ActiveRecord
{
  public string Name { get; set; }
  public int AssignedTo_UserId { get; set; }
  public DateTime DueOn { get; set; }
}

public class NotificationTask
{
  Task task;

  public NotificationTask(Task t)
  {
    this.task = t;
  }

  public void Save()
  {
    bool isNew = t.IsNewRecord;
    t.Save();
    if (isNew)
      Email.Send(...);
  }
}

public class TasksController : Controller
{
  public ActionResult CreateTask()
  {
    ...
    new NotificationTask(new Task {...}).Save();
    ...
  }
}

This is not really the decorator pattern... At least not as defined by the GoF, but I have seen it used this way often enough that I don't feed too terrible calling it that. Really this is just a wrapper class. It's similar to the inheritance approach, except because it doesn't use inheritance, it opens us up to use inheritance on the Task for other reasons, and apply the email behavior to any kind of task.

The naming is a bit suspect, because NotificationTask is not really a task, it just has a task. It implements only one of the task's methods. If we extracted an ITask interface we could make NotificationTask implement it and just forward all the calls. This would make it a task (and a decorator), but would also be crazy tedious.

Service

public class Task : ActiveRecord
{
  public string Name { get; set; }
  public int AssignedTo_UserId { get; set; }
  public DateTime DueOn { get; set; }
}

public class CreatesTask
{
  Task task;

  public NotificationTask(Task t)
  {
    this.task = t;
  }

  public void Create()
  {
    t.Save();
    Email.Send(...);
  }
}

This service represents the standard domain behavior for creating a task. In an edge case where you needed a task but didn't want the email, you would just not use the service.

The naming is pretty nice here, hard to be confused about what CreatesTask does... However, this path leads to a proliferation of <verb><noun> classes. In the small it's manageable, but as they accumulate, or as they start to call each other things get confusing. For example, if you know nothing about Task and you have to start working on it, would you know you should call the CreatesTask service? Would you know it exists? And would you be sure it was the correct service for you to be calling?

Dependency Injection

public class Task : ActiveRecord
{
  public string Name { get; set; }
  public int AssignedTo_UserId { get; set; }
  public DateTime DueOn { get; set; }

  INotifier notifier;

  public Task(INotifier notifier)
  {
    this.notifier = notifier;
  }

  public override void Save()
  {
    bool isNew = t.IsNewRecord;
    t.Save();
    if (isNew)
      notifier.Send(...);
  }
}

public class TasksController : Controller
{
  public ActionResult Create(...)
  {
    ...
    new Task(new EmailNotifier()) { ... }.Save();
    ...
  }

  public ActionResult CreateWithNoEmail(...)
  {
    ...
    new Task(new NullNotifier()) { ... }.Save();
    ...
  }
}

I'm going to ignore all the complexity around the fact that this is an ActiveRecord object which the ActiveRecord framework will usually be responsible for new-ing up, which makes providing DI dependencies difficult if not impossible...

The idea here is to pass in an INotifier, and then when you find yourself dealing with a task you'll build it with the notifier you want it to use.  If you want no notification, you use the Null Object pattern and pass in an INotifier that doesn't do anything (called NullNotifier in the code example).

But this has the ORM-framework draw back I mentioned above.  Plus it requires the code that is constructing the task to know what behavior the code that is going to save the task will require.  Most of the time that's probably the same code, but if they aren't, you're out of luck.

Operational vs Data Classes

public class TaskInfo
{
  public string Name { get; set; }
  public int AssignedTo_UserId { get; set; }
  public DateTime DueOn { get; set; }
}

public class TaskList
{
  public TaskInfo Create(TaskInfo t)
  {
    t.Save();
    notifier.Send(...);
    return t;
  }
}

Here I've separated the data class from the operational class. I talked about this in the Stratified Design series of posts.  This separation hides ActiveRecord, giving us the control to define all of our operations independently of the database operations they may require.  If we needed to save a task without sending an email we could just call TaskInfo.Save() directly from whatever mythical operation had that requirement.  Or we could do some extract method refactorings on the Task.Create method to expose methods with just the behavior we need.  Or we might extract another class.  Naming is going to be hard for these refactorings, but at least we have options.

If I missed anything, or if you see an important variation I didn't think of, please tell me about it!  As always you can talk to me on twitter, and you can still fork the original gist.