From collaborator hell to dependency-free bliss

Let's say you're a developer on a C# app and just got a batch of new requirements. Let's also say that it has a fairly simple requirement: when a user closes his or her account, the account status should be set to closed. You, as a good Test Driven Developer, write your unit test, watch it fail, then add the following code to the Account class and watch your test pass:

public void Close() {
  this.Status = AccountStatus.Closed;
}

Easy-peasy.

Many weeks later you come across a requirement that says when a user closes his or her account, the account owner's IsSpecial flag should be set. To make your latest unit test pass, you do the simplest thing that could possibly work.

public void Close() {
  this.Status = Status.Closed;
  this.Owner.IsSpecial = true;
}

Your test went from Red to Green and now it's time to Refactor. Having your Account mutate some value on another object breaks encapsulation so you Control+R & E to Extract Method then Control+R & M to Move your method to the Owner class and give it a clear name.

public void Close() {
  this.Status = Status.Closed;
  this.Owner.ClosedAccount(this);
}

That's a little better. In stead of just setting some property you're now telling the other object that something happened. It's free to handle that how it wants and the Account needn't know about it - a textbook case of encapsulation and The Hollywood Principle.

You've worked in this domain long enough to know that all kinds of things are going to happen when an account is closed. Maybe the other things should be moved to a different method? Not entirely YAGNI, but modifying the account and telling others about it are two different things so you could make the argument that they should be two separate methods.

public void Close() {
  this.Status = Status.Closed;
  this.OnClosed();
}

private void OnClosed(){
  this.Owner.ClosedAccount(this);
}

It's a judgment call but this does clearly separate things. There's one method where the account modifies itself and another where all the other objects get to react to the account being updated. You know that if you have requirements that start with when an account is closed, then the code that deals with the account should go in the Close method and code that deals with other classes goes in the OnClosed method. But wait a second - doesn't this sound familiar? There's even something built into the C# language for just this scenario: events and delegates.

public delegate void ClosedEventHandler(object sender, AccountClosedEventArgs e);

public class Account {
  public event ClosedEventHandler Closed;

  public void Close() {
    this.Status = Status.Closed;

    if (this.Closed != null)
      this.Closed(this, new AccountClosedEventArgs(this));
  }
}

Now the owner can register a callback with the account's Closed event and the account can just tell whoever's listening when it has closed. The Account class no longer needs to reference the Owner class and we can rejoice.

But something about this still doesn't quite seem right. Now instead of the Account telling the Owner, the Owner has to listen to the Account. You still have one class directly referencing and depending on another - you just switched the dependency direction. A day or two later you remember you once read something about Messaging and Domain Events so you look it up and get that started.

public void Close() {
  this.Status = Status.Closed;
  EventBus.Publish(new AccountClosed(this));
}


/* ... elsewhere ... */
public class AccountClosed{
  public Account Account { get; set; }

  public AccountClosed(Account account){
    this.account = account;
  }
}


/* ... elsewhere ... */
class AccountEventsHandler {

  private IRepository repository;

  public AccountEventsHandler(IRepository repository){
    this.repository = repository;
  }

  public void Handle(AccountClosed e){
    this.repository.GetOwner(e.Account).IsSpecial = true;
  }
}


/* ... elsewhere ... */
public void WireEverythingTogether(IRepository repository){
  var accountHandler = new AccountEventsHandler(repository);
  EventBus.Subscribe<AccountClosed>(accountHandler.Handle);
}

Much nicer. Not only is the dependency completely removed from the Account and Owner classes, but each class no longer exposes methods that are just used for reacting to each other. The Account only modifies itself and creates events and the Owner only modifies itself and creates events. Now you have the Account depend on the EventBus and somewhere else you subscribe other classes to the appropriate events. You have a simple AccountEventsHandler that deals with coordinating other objects when the account changes so the domain objects themselves don't need to. It can be little bit more work, but it does reduce coupling and allow much cleaner domain objects. But the Account is still referencing the EventBus directly. You could use a service locator:

public void Close() {
  this.Status = Status.Closed;
  Services.Get<IEventBus>().Publish(new AccountClosed(this));
}

But the service locator can cause problems - like if an IEventBus wasn't registered - and is considered an anti-pattern by some people. You could use constructor injection to pass in the dependency instead.

public Account(IEventBus bus){
  this.eventBus = bus;
}

public void Close() {
  this.Status = Status.Closed;
  this.eventBus.Publish(new AccountClosed(this));
}

That's clean and a good use of the Dependency Inversion Principle, but maybe overkill since the EventBus is so simple and ubiquitous. Maybe hard coding a such a simple dependency is ok. In my limited experience this is an acceptable time for bending the rules and relying on a statefull static class. You will probably never hear me say that again since I hate working with static classes and singletons that have state or complex behavior. On the other hand, constructor injection is a great first step to using a Dependency Injection Container to wire up the EventBus and other dependencies.

Maybe months from now you can switch to actual full-scale event sourcing and CQRS. With command handlers, the Domain Objects create events and store them locally. Then the command handler would fetch all the events from the domain objects and pass them to the event bus; your domain objects don't even need to know about the event bus anymore since the command handler deals with that.

---

This series of slightly different ways of dealing with different objects involved in the same action is something that I've had a lot of problems with. It's all the same basic result but the important difference is in how to deal with coordinating different objects when something happens. These different designs have very different benefits and costs. The earlier examples are easy for anyone to do, easy to see exactly what happens when debugging or looking at the code, and you know exactly where to make the change: right in the method itself. The downside is that it becomes collaborator hell and all your classes quickly become so bloated that most of the class's code is actually about other classes. The classes become so interdependent that you can't change one thing without having to change everything else. Having lots of dependencies like this can also make build times suffer too. The latter examples only rely on the Event Bus, Service Locator, or Inversion Of Control Container so they are as non-dependent as possible but I can't really tell what's going to happen when the program runs. If everything is wired together with configuration in xml files or sql tables then it can also become exceedingly tedious and error prone to change. This kind of configuration becomes especially difficult if the developer's environment, qa environments, and production environments can all be configured differently. Not everyone will be familiar with or even like the event-based designs either. The whole thing is made even more confusing if different sections of the project have different ways of doing things and people mostly just do whatever they copied and pasted from - not that I've ever done that of course....