Tuesday, December 21, 2010

The Second Commandment: Thy entity shalt not participate in dependency resolution

Following on from my First Commandment a while back, I’d like to talk about a second mistake that a lot of people make when getting started with IoC containers. They worry about getting their domain entities sourced from the container, or participating in dependency resolution in some way. Domain entities should be POCO classes, they shouldn’t participate in dependency resolution.

Most business applications have ‘domain entities’ AKA ‘business objects’ usually something like this:

public class Car
{
public void Start()
{ ... }

public IEnumerable<Wheel> Wheels
{
get { return ...; }
}
}

When you start using an IoC container it’s a natural reaction to think, ‘how do I get my Car from the container’ or ‘how can my Car’s dependencies be resolved from the container’. You might see code like this for example:

public class Car
{
public IWheelRepository WheelRepository { get; set; }

public IEnumerable<Wheel> Wheels
{
get { return WheelRepository.GetWheelsFor(this); }
}
}

Along with code like this:

var car = container.Resolve<Car>();

This is wrong.

The general rule is that the domain model shouldn't have any outward facing dependencies, so you shouldn't be asking them to reference components from the container. The worst example of this is the Active Record pattern where domain entities are also responsible for their persistence. If you have a Customer object with methods like 'Save' and 'Update' you are breaking this rule.

The persistence problem is easy to solve, simply de-couple your domain class from its persistence. You now have a Customer and a CustomerRepository or maybe an IRepository<Customer>, I like the last one. Modern ORMs like NHibernate make this easy to do.

But what if your business logic requires you to do something with the 'outside world', like send  an email?

Say I have an Order domain entity that has a rule: when the order is confirmed, send the customer a confirmation email. I probably feel like I want to do something like this:

public class Order
{
private readonly IEmailSender emailSender;

public Order(IEmailSender emailSender)
{
this.emailSender = emailSender;
}

public void Confirm()
{
// ...
var confirmationEmail = CreateConfirmationEmail();
emailSender.Send(confirmationEmail);
}
}

But if my Order is not resolved from the container, how can I inject the IEmailSender?

There seem to be 3 main ways people solve this problem, each with its pros-and-cons...

1. Factor your business logic into a domain service, so I now have an Order class and an OrderService with a Confirm(Order order) method, the OrderService can be resolved from the container and have a dependency on the IEmailSender.

public class OrderService
{
private readonly IEmailSender emailSender;

public OrderService(IEmailSender emailSender)
{
this.emailSender = emailSender;
}

public void Confirm(Order order)
{
// ...
var confirmationEmail = CreateConfirmationEmail();
emailSender.Send(confirmationEmail);
}
}

The problem with this, is that we're now loosing encapsulation, our Order class' functionality is leaking out into domain services and it's ending up being just a property bag; the anaemic entity anti-pattern. We're not doing object oriented programming any more.

2. Inject the IEmailSender into the Order's confirm method: Order.Confirm(IEmailSender emailSender). Now we can get the IEmailSender from the container by doing constructor injection on the orchestrating class and simply pass it in when we call Confirm.

public class Order
{
public void Confirm(IEmailSender emailSender)
{
// ...
var confirmationEmail = CreateConfirmationEmail();
emailSender.Send(confirmationEmail);
}
}

The problem now is that the dependencies of the Confirm operation have become the responsibility of the orchestrating class and it has to worry about getting an instance of IEmailSender from somewhere. This violates Separation of Concerns. It makes your Confirm method brittle as well, what happens if you decide you don't need to send an email at a later date? Do you then go and rewrite every consumer of Order?

3. Udi Dahan's domain events pattern or something like it. Here we raise an event from our Order class' Confirm method and then have a handler that sends the email. Because the handler is resolved from the container, it can have the IEmailSender passed in via constructor injection. The caller is unaffected, it just calls Order.Confirm() and we don't have domain logic leaking out of the Order entity.

public class Order
{
public void Confirm()
{
// ...
DomainEvent.Raise(new OrderConfirmedEvent(this));
}
}

public class SendEmailOnOrderConfirmHandler : IHandle<OrderConfirmedEvent>
{
public void Handle(OrderConfirmedEvent orderConfirmedEvent)
{
var confirmationEmail = CreateConfirmationEmail();
emailSender.Send(confirmationEmail);
}
}

The downside is that we now have a nasty static dependency on the DomainEvent.Raise call scattered throughout our domain model.

I've used all three of these techniques at different times. I tend to avoid 2, it's got the nastiest code smell, and I'd probably favour 3 these days, simply because I always tend to have an Entity super class that can wrap the DomainEvent.Raise call and I really like the emerging CQRS patterns that are closely related.

But whatever you decide to do, avoid getting into the trap where you try and resolve your domain entities from the container.

9 comments:

Harry M said...

For the reasons you've touched on, 2 is unmaintainable and clearly leads to the

Option 3 is a disguised service locator - the DomainEvent class has to resolve some service to deal with the raised message.

I strongly feel that in this case option 1 is the best (and has the weakest argument against it). You can keep the SRP by naming your service something like OrderEmailSender instead of the generic SomethingService (the postfix 'Service' is a warning sign already).

Injecting the services can be done very cleanly these days with NHibernate by implementing a custom proxy factory
http://nhforge.org/blogs/nhibernate/archive/2008/12/12/entities-behavior-injection.aspx, and I think that it was just the difficulty in doing this historically that made people worry about injecting services into entities.

Dave said...

I agree with Harry but that maybe just from not understanding the problem you have defined. Requiring the email to be sent is a business rule so I wouldn't package that with the domain object but make it part of the service. So what are the rules for partitioning business rules into services, pocos, and dtos?

Maciek said...

How about a combination of 2 and 3:

class Order
{
  public void Confirm(IEventPublisher publisher)
  {
    // ...
    publisher.Publish(new OrderConfirmedEvent(this));
  }
}

We get the benefits of DomainEvents and avoid static field.

Mike Hadlow said...

@Harry,

Thanks for the link, I hadn't seen that. It's an interesting technique, but I still think that there are some problems. What about initial entity creation? Getting the initial entity from the container feels wrong to me, it means you have to create the entity in a generic state and then populate it with its property values afterwards. A common pattern is for a domian entity to create new entities, Customer.CreateOrder() for example, you wouldn't be able to do that without referencing some kind of service locator inside the Customer entity.

@Dave,

IMHO Entities should encapsulate your business rules and processes. Domain services should support the entities, but avoid taking over from them. DTOs are simply data buckets to transport state from one place to another.

@Maciek,

Hmm, I think doing that has the same problems as solution 2, it makes the Confirm method brittle and makes too many expectations of Confirm's caller.

Szymon Pobiega said...

I don't agree with your opinions. Version 2 (sometimes referred as double dispatch) is quite good. If used, application layer service is responsible for passing the dependency in parameter. It can get this dependency injected by IoC infrastructure.

I don't like version one (which is injecting services into entities). I wrote on this subject here: http://simon-says-architecture.com/2010/03/31/dependency-inversion-patterns-and-the-domain-model and Jimmy Bogard put his thought here http://www.lostechies.com/blogs/jimmy_bogard/archive/2010/04/14/injecting-services-into-entities.aspx and here http://www.lostechies.com/blogs/jimmy_bogard/archive/2010/03/30/strengthening-your-domain-the-double-dispatch-pattern.aspx.
DomainEvents version uses service locator internally but I don't see anything bad in using it in such a case.
Last but not least, the solution which seem to be the most elegant (but not applicable in all cases) is event sourcing. It makes event publication and subscription first class concepts and by doing this avoids this 'nasty' static dependency.

Maciek said...

I'm going to argue that IEventPublisher in double-dispatch scenario is better then IEmailSender.

Caller does not need to know how the event is going to be processed or even what event is going to be raised (if it is going to be raised at all).

Passing IEventPublisher in such context might become a "standard" way for invoking methods on domain classes.

Another option I've seen proposed somewhere was to have thread-static, "scoped" publisher accessible through a static property or method, but I think I'd prefer an explicit dependency on the publisher.

Harry M said...

Mike, if a customer is going to create orders its going to need an OrderFactory (or even a Factory if you don't need any sugary factory methods).

The problem with any event publisher is that it doesn't allow for a result to come back to the thing that raised the event. An event bus allows you to decouple a class from a knock-on event, but they don't allow you to get your result from your TaxCalculator.

if you are using frameworks with sufficient IOC integration (MVC3 mostly, NHibernate) where you can inject at any point where the framework instantiates a class for you, you should be able to hide a classes dependencies from other classes that don't need to know about them using Factories and never have to reference a static, be it a ServiceLocator or a DomainEvent class.

ryzam said...

Hi Mike

Right now I've moved most of my project to follow DCI and from my understanding, this can be resolve easily in DCI.

Dariusz Tarczyński said...

How solution 2 is different from Strategy Pattern? Isn't the same?