Friday, July 31, 2009

Global state is rarely a salvation

This post is a response to Domain Events - Salvation from Udi Dahan, where he shows a design to manage the Domain Events pattern. He suggest to use a static class to handle publishing and subscribing, but I argue that a static class solution is not pure as it seems.

Disclaimer: Udi Dahan is an authority in DDD and enterprise application development. I think it is a person who gets things done and I do not contest the validity of his approach in production environments. Active Record often gets the job done also and I used it a lot in the past, but there are cleaner solution arising.

In the Domain Events - Salvation post, Udi proposes the third reworking of his Domain Events implementation. DomainEvents is a pattern similar to Observer or Publish/Subscribe applied to a Domain Model, where the domain objects are observed or observers.
He sustains that you should not never inject anything in Entities, and I agree since Entities are not injectables. Here's how he raises an event:
public class Customer
public void DoSomething()
DomainEvents.Raise(new CustomerBecamePreferred() { customer = this; });
Then he says:
We’ll look at the DomainEvents class in just a second, but I’m guessing that some of you are wondering “how did that entity get a reference to that?” The answer is that DomainEvents is a static class. “OMG, static?! But doesn’t that hurt testability?!” No, it doesn’t. Here, look:
and he proceeds to show a (not) unit test where an event is raised.
Now, let's clarify some points:
  • a static class is more or less a singleton. They both have reset-like methods and a unique copy of data globally accessible wherever you want. You can make the class package-private in some languages, but every class have potential access to the singleton instance.
  • Singletons are pathological liars. They hide dependencies and carry global state around, between tests.
  • This solution uses a static class.
We can conclude that this Domain Events implementation is not a beautiful architecture as it was proposed. Let's explore the problems it raises:
  • If you forget to reset() the static class between tests, the test suite can blow up suddenly in a strage place and the fault will be hard to locate. An instance can fire an event that is forwarded to subscribers that do not exist anymore. This is global state in action.
  • Everytime you test an entity class, you're testing also DomainEvents class, since it cannot be mocked.
  • There is a compile-time dependency on DomainEvents. In other languages like Php this would not be a problem since there's no compiling, but Udi's examples use .NET and Customer is now dependent on DomainEvents. A possible solution would be defining events support directly in the language, but it is a big shift in the object-oriented paradigm.
  • Production code will be an action at a distance.
Let's explore the last issue.
The code of a service or controller layer will be similar to:
// in the setup
b => creditCardProcessor.charge(b.Customer, b.Amount)
// Customer class, an Entity
public class Customer
public void bill(int amount)
DomainEvents.Raise(new CustomerBilled() { Customer = this; Amount = amount });
// some controller, client code
Customer c = new Customer();
c.bill(1000); //someone is billed? but who? what happens when I call this? if I not persist this customer instance nothing has happened, right? No.
compare it with:
billingService.charge(c, 1000);
c.bill(billingService, 1000);
The billing service does not lie: it requires a CreditCardProcessor in the constructor. Now I see what happens: the credit card of the customer is charged for 1000.
My solution is always the same: if a method of an entity requires an injected dependency, place it on a service class or ask for the dependency in the method parameters, extracting an interface to put in the signature. Asking in the constructor is fastidious since we want to be able of create stateful objects like entities simply by calling new, or whenever we create one we have to ask for a factory.

My conclusion is that using a static class in an entity class and say that you're not injecting anything is right, because you're not applying Dependency Injection at all. The phrase:
The main assertion being that you do *not* need to inject anything into your domain entities.
should be changed to The main assertion being that you do *not* need to inject anything into your domain entities: simply throw in a static class so that your entity stops asking for things and begin looking for things, abandoning Inversion of Control/Dependency Injection.
Again, I do not question that this design gets the job done. But don't say "I'm not injecting anything, that's good" if you're nail down a lightweight newable class to a static infrastructure one.


zampano said...

>Customer c = new Customer();
>c.bill(1000); //someone is billed? but who?

Huh? Who?
Well, if you create entities with no identity (bad) or aggregates (evil), then you have to ask yourself.

Didn't you recommend to always (hm..) use factories for newing up?

Giorgio said...

I use factories for service objects, that has dependencies on other collaborators. The idea is that a factory's responsibility is to assemble part of an object graph, and a object like a customer (Entity or Value Object) has to be a leaf of this graph with no dependencies. I explained it in: