Simplify code by using composition

In my post about software simplicity I talked about how we introduce complexity in software without realizing it. In this post I want to discuss a pattern that allows you to get rid of the complexity introduced by using annotations or aspect oriented concepts. Let’s take a look at a basic example:

public class TaskService
{
    private TaskRepository repo;
    public TaskService(ITaskRepository taskRepository)
    {
        repo = taskRepository;
    }

    public void CreateTask(string name)
    {
        repo.Insert(new Task(name));
    }

    public void MarkTaskDone(int taskId, string userName)
    {
        var task = repo.Get(taskId);
        task.Status = TaskStatus.Done;
        task.ModifiedBy = userName;
        repo.Update(task);
    }
}

This class has two small methods to create and update a task. Until now, this code is not complex and is fairly straightforward. Since these methods map one to one with use cases, they’re a good place to deal with cross-cutting concerns such as logging, authorization and exception handling.

Implementing cross-cutting concerns with annotations

A common solution is using annotations or aspect oriented programming. This allows us to describe how code behaves:

public class TaskService
{
    [Log]
    [IgnoreExceptions]
    [Authorize("admin")]
    public virtual void CreateTask(string name)
    {
        repo.Insert(new Task(name));
    }

    [Log]
    [LogExceptions]
    [Authorize("admin,user")]
    public virtual void MarkTaskDone(int taskId, string userName)
    {
        var task = repo.Get(taskId);
        task.Status = TaskStatus.Done;
        task.ModifiedBy = userName;
        repo.Update(task);
    }
}

Apart from the attributes, the code looks equally simple and although it’s still as readable as it was before, it’s a lot more complicated. For those attributes to do their work, we need something to intercept calls to these methods. To intercept the calls, we need to make sure that anything that calls these methods, gets rewritten (either at compile time or dynamically at runtime).

This can be done by using a dynamic proxy (there are other ways, but none of them are really simple). A dynamic proxy is a runtime generated class, that inherits from your class and overrides its methods. In the overridden method, depending on the implementation of the aspect, it can do something before or after the original method or something entirely different than what the method was actually supposed to do. That’s a lot of hidden complexity:

  • A runtime generated class
    That’s definitely not simple
  • Inherits from your class and overrides the method
    This implies that you define your method as virtual. This is something you should just “know
  • It can do something before or after the original method or something entirely different than what the method was actually supposed to do
    From a client’s perspective, this is unexpected behavior. When you look at the CreateTask-method, at first sight there’s nothing to it: it creates and inserts a new Task, true to the name of the method. With the annotations, that’s not really true anymore: it now logs all calls (how? where? before? after?), ignores exceptions (all of them? does it log them?) and it authorizes the call (what happens when auth fails?).

So these three little annotations actually introduce a lot of complexity, while still keeping our code deceivingly easy.

The problem

Although the annotations introduced complexity, they still add value: we are not mixing cross-cutting concerns with domain logic. So is this complexity a necessary evil or is there a better solution? To find a better solution, we first need to find out what the problem is.

What we want to do is add behavior on top of an existing method. Why can’t we do that with plain composition? Do we need to use runtime generated code?

The problem is that we don’t have a common interface. If there was one, we could get rid of the dynamic proxies and annotations and just build an explicit object graph (using a decorator pattern for example). A dynamic proxy solves this by creating a common interface at runtime in the form of a PointCut. Because there’s no common interface at compile time, it generates one at runtime using reflection, so you can write an attribute implementation like this:

public void InterceptMethodCall(PointCut calleeInfo)
{
    var methodName = calleeInfo.Name;
    var passedParameters = calleeInfo.Paramters;
    ....
    // Implement security, logging, ... here
}

This method can now be attached to any other method. A dynamic proxy enables us to convert any type of method-signature into a PointCut. Or phrased differently: a dynamic proxy enables us to convert any method into a message.

Changing the problem

Solving this problem is impossible with simple composition (using today’s languages as far as I know). So, to come up with a simpler solution, we first need to change our problem. Instead of dealing with N different methods that have N parameters, we could say that we’re only going to have a method that has 1 parameter. To do so, we need to apply a small refactoring: “Introduce parameter object” (Fowler, Refactoring, improving the design of existing code). This is how our code looks like after applying the refactoring:

public class TaskService
{
    private TaskRepository repo;
    public TaskService(ITaskRepository taskRepository)
    {
        repo = taskRepository;
    }

    public void Handle(CreateTask command)
    {
        repo.Insert(new Task(command.Name));
    }

    public void Handle(MarkTaskDone command)
    {
        var task = repo.Get(command.TaskId);
        task.Status = TaskStatus.Done;
        task.ModifiedBy = command.userName;
        repo.Update(task);
    }
}

Instead of having methods with N parameters, each method now just accepts one parameter. That parameter is an object which holds all the parameters w epreviously had and implements an interface ICommand. (ICommand is just a marker interface, it doesn’t have any properties or methods).

The solution

Since all our methods are now the same, we can extract a common interface:

public interface ICommandHandler<T> where T : ICommand
{
    void Handle(T command);
}

and implement it in our class:

public class TaskCommandHandlers : ICommandHandler<CreateTask>,
                                   ICommandHandler<MarkTaskDone>
{
    private TaskRepository repo;
    public TaskService(ITaskRepository taskRepository)
    {
        repo = taskRepository;
    }

    public void Handle(CreateTask command)
    {
        repo.Insert(new Task(command.Name));
    }

    public void Handle(MarkTaskDone command)
    {
        var task = repo.Get(command.TaskId);
        task.Status = TaskStatus.Done;
        task.ModifiedBy = command.userName;
        repo.Update(task);
    }
}

Because we have a common interface we can use composition to implement our solution. This is an example of how you could implement logging:

public class LogHandler<T> : ICommandHandler<T> where T : ICommand
{
    ICommandHandler<T> _next;
    public LogHandler(ICommandHandler<T> next)
    {
        _next = next;
    }
    public void Handle(T command)
    {
        var logMessage = serialize(command);   // pseudo code
        log(logMessage);                       // pseudo code
        _next.Handle(command);
    }
}

The LogHandler implements the same interface, (and thus can replace the original implementation). We accept another instance of the same interface which we’ll invoke after logging the message. This is a basic example of the decorator design pattern.

To use this, you could either wire it up manually:

ICommandHandler<CreateTask> handler = new LogHandler<CreateTask>(
						new TaskCommandHandlers(
							new TaskRepository()
						)
					    );
handler.Handle(new CreateTask("TaskName"));

Or you could use any other method of composition. This is an example of how to wire this up with a DI container (Ninject):

kernel.Bind<ICommandHandler<CreateTask>()
      .To<TaskCommandHandlers>()
      .WhenInjectedInto<LogHandler<CreateTask>>();

kernel.Bind<ICommandHandler<CreateTask>()
      .To<LogHandler<CreateTask>>();

Conclusion

By changing our problem we can eliminate complexity. The advantages of the proposed solution over an annotation based solution:

  • No dependency on an external framework (bugs, discontinuation, learning the new framework, …)
  • Simpler code, using plain composition instead of runtime code generation
  • No implicit design constraints (virtual methods, …)
  • Single responsibility: when we apply annotations to a class, we’re violating SRP (single responsibility principle): even though it’s metadata, the class is still responsible for having the correct metadata about logging, exception handling, … If we extract this into a composeable solution like we did, we can now configure this behavior at a higher level and consolidate it.
  • Testability: using annotations, you’d have to run the application to test that your pipeline works as expected (that means end-to-end testing). With simple composition, you could write an integration test (in the form of a unit test) that creates the pipeline and verifies the results

Using this pattern is just one example of how we can keep our code simple and prevent complexity from creeping in. This is not easy though. Simple != easy, (often the opposite is true) but it’s well worth it in the long run. Rich Hickey has an excellent presentation on the difference between simple and easy which I recommend watching: http://www.infoq.com/presentations/Simple-Made-Easy

  • daniel rosales

    kenneth, can you provide the entire code base for the concepts surrounding this solution? Thanks, @iamsystem

    • http://www.kenneth-truyers.net/ Kennethtruyers

      Hi Daniel,

      I’ll check whether I can create a small GitHub repo. I’m planning on doing a few more related posts with patterns, so it’s possible I’ll do it afterwards. I’ll make sure to send you a comment when it’s up.

  • http://www.github.com/darrencauthon darrencauthon

    I have some experience in this method, and I did it for the same reasons: SRP, cleaner code, fewer dependencies, not having to stick on annotations, yadda yadda yadda. I think it works well to start, and it’s ok for one or two overrides, but after that it starts to fall apart.

    The IoC registrations are the easiest thing to highlight. We added logging to this method, but let’s say we want to add behavior [A]? Now behavior [B]? Now behavior [C]? Now imagine what that Ninject code would look like… not so friendly. Sure, Ninject has neat “fluent” interfaces for handling this sort of thing, but those interfaces are specific to the domain of dependency injection – not to the domain of my app. This may mean a block of incomprehensible IoC registrations that *somehow* wire up in the way you want your app to run. The annotations don’t have to have this problem, as they’ll be written in the language of the app’s domain.

    An easy thing to say here is to say, “Well, when we get that far we’ll do something different.” But we’ve already paid a cost in creating abstractions specific to this method, and you’d probably have dropped them all over your app.

    After having failed with this approach a few times, I think the first trick I’d reach for is to abstract on the level of handling the events… but not on the idea that each one calls the next. Leave that to the abstraction, and let each one focus on its behavior alone. The registration would have to be as simple as registering that says: When occurrs, run through this list: [Logging, A, B, Saving]. Or something to that effect. Keep these behaviors together in an array, not a fluent IoC registration interface.

    • http://www.kenneth-truyers.net/ Kennethtruyers

      I would personally not use a container, if you plan to do manual registration, but just wire everything by hand. I know, you end with maybe a few 100 lines of composition code, but that’s very simple code and you can see how your complete application is composed in a single file. That also makes lifetime troubleshooting a lot easier. I was going to write an article about that, but Mark Seemann already did so, and probably better than what I could have done: http://blog.ploeh.dk/2014/06/03/compile-time-lifetime-matching/

      That is, if you don’t have very strong conventions (ie: everything in the domain needs behavior A, B and C on top of it). In that case you can use a container to auto-wire the whole thing based on conventions.

      • http://www.github.com/darrencauthon darrencauthon

        You say: “I know, you end with maybe a few 100 lines of composition code, but that’s very simple code and you can see how your complete application is composed in a single file.”

        As someone who has said the same thing… those 100 lines of composition code are not “simple” and the visibility is not as great as we think. Look at the “manual” code you wrote to do the registration: That’s not readable. I think we trick ourselves into thinking so, as we’re probably experienced in C# and our heads are deep in the domain when we wrote it, but it’s very difficult to read. It’s just a blur of generics and constructors.

        And with the blur, there are other things that could go wrong. A slight twist of the wrist, a parenthesis here or there in the wrong place, the lifetime manager put in the wrong spot… uh oh, it won’t behave in the way we want, and nobody will know it because nobody can read it.

        I don’t think that loss of readability is necessary — it just takes a shift in the abstraction. Abstract the behavior, not the code. :)

        • http://www.kenneth-truyers.net/ Kennethtruyers

          Well, I think if that codes becomes too complex, then you’re dependency hierarchy is probably too deep, which is another benefit of doing it manually, you’ll actually feel the pain of having to type that all the time. Instead of going for a complex solution, which is easy to implement, you should probably think about why you have such a deep dependency graph.

          As for the lifetime manager: new => transient , inside method => per request, inside class => singleton. I’m not sure how that can get complicated.

          The thing is that this is just plain c# code (I agree that generics and constructors can become a blur), but it’s still a lot easier then dynamically generated proxies, which you don’t even see, let alone debug them properly

          • http://www.github.com/darrencauthon darrencauthon

            I have a deep dependency graph because I’m using the practice you’re describing as an alternative to the attributes. And the pain I’d be feeling is due to the method you’re describing here.

            It’s not very deep at the level described in a blog post, but I believe it will quickly turn when a few changes come through. Now instead of an ICommandHandler inside an ICommandHandler, now I’ll have an ICommandHandler inside an ICommandHandler inside an ICommandHandler inside an ICommandHandler. If I’m going to feel this pain after three changes, why walk in this route after the first change?

            You say “debug them properly,” maybe that’s where the rubber will meet the road. What would be more interesting is a comparison of not just the options, but the differences in the tests they’d result in. I’d love to drop a few feature requests into this setup, with failing tests, and see where it goes. :)

          • http://www.kenneth-truyers.net/ Kennethtruyers

            What I’m saying is that if you have five nested CommandHandlers, your problem is probably somewhere else and there’s possibly a better way to solve the issue. Granted, that’s a bit of discussion in thin air without a real world example.

            For testing it depends on what kind of tests you want to do. This setup allows you to do pure unit tests: test a single commandhandler, or an integration test: construct the pipeline and check what comes out at the end.

            I do agree that this not the easiest road to take, and it will take more time getting started (because you need to build some infrastructure), but in the long run, there are a lot of benefits:
            * You don’t need to learn yet another framework. This is also beneficial when other programmers join the team. If there’s an AOP framework and a DI-container and an ORM and a mapper and a … new members will have get up to speed will all these before they can actually be productive. If you can use plain composition, they’ll just have to learn the application architecture, which they have to do anyway + all the code is explicit so it’s even easier to see what the application does.
            * You have all the code, that means that there’s no hidden complexity. If there is complexity, at least it’s visible.
            * Since you have all the code at hand, it also means that you can lean on the compiler to build your graphs. With annotations, it could blow up at runtime because of misconfiguration.

          • http://www.github.com/darrencauthon darrencauthon

            I think I’d like to be in the same notification boat as daniel rosales… if you ever get an example repository on Github, I’d love to see it.

  • Ryan

    I’d lean more towards your annotation solution. It’s very clear what behavior occurs and it has a high signal to noise ratio. Some of the worst complexity is invisible complexity and stuff that “magically happens”, which we are in danger of doing anytime we over use IoC. Though I agree in theory with your solution and the problems it solves, I find in practice the former solution to be more practical.

    • http://www.kenneth-truyers.net/ Kennethtruyers

      I think I can almost provide the same answer as I provided to Darren :-) IoC was an example here of how you could wire it. I’d personally wire it by hand. In a single composition root, just write out and compose your entire app: simple and very explicit. Yes, it will take you some typing to wire the whole thing, but I don’t think typing is the bottleneck.

  • fahed

    Slight typing error:

    public class TaskCommandHandlers : …
    {

    public TaskService(ITaskRepository taskRepository)

    }

    Constructor doesn’t match class name. Otherwise, excellent article!