DI and events: Constructor Subscription by Mark Seemann

Using Constructor Injection, you can subscribe to an event within the constructor, but should you?

This post is the first in a series of articles about Dependency Injection and events.

Imagine that you have an interface that defines an event:

public interface IDependency
{
    event EventHandler ItHappened;
}

In order to keep the example simple, the ItHappened event carries no information; it just raises an event with an EventArgs instance. Thus, subscribers can react to the fact that this particular event happened, but there's no data contained in the event. However, the following discussion doesn't change if the event carries information.

A class must react to the events raised by IDependency implementations. A common approach is to subscribe to the event in the constructor. We can call this pattern Constructor Subscription:

public class NeedyClass
{
    private readonly IDependency dependency;
 
    public NeedyClass(IDependency dependency)
    {
        if (dependency == null)
            throw new ArgumentNullException("dependency");
 
        this.dependency = dependency;
        this.dependency.ItHappened += this.OnItHappened;
    }
 
    private void OnItHappened(object sender, EventArgs e)
    {
        // Handle event here
    }
}

The concern is that, by subscribing to an event, the constructor violates Nikola Malovic's 4th law of IoC: Injection Constructors should perform no work.

This rule about Dependency Injection explains why you can compose big object graphs with confidence. Still, the most compelling reason for conforming strictly to the rule is most likely performance considerations. So, what if you subscribe to a single event or two during construction? Will it adversely (and noticeably) affect performance of your overall system?

As always, the answer to such questions is: measure. However, I'd be quite surprised if it turns out that a single event subscription has a huge impact on performance...

Smell #

Consider the implementation of NeedyClass: it contains a design smell. Can you spot it?

Given the definition of the IDependency interface, the ItHappened event is the only member defined by the interface. If this is the case, then why is NeedyClass holding on to a reference of the interface?

You can remove the dependency field from NeedyClass, and nothing is going to break:

public class NeedyClass
{
    public NeedyClass(IDependency dependency)
    {
        if (dependency == null)
            throw new ArgumentNullException("dependency");
 
        dependency.ItHappened += this.OnItHappened;
    }
 
    private void OnItHappened(object sender, EventArgs e)
    {
        // Handle event here
    }
}

From the perspective of an outside observer, that's really strange. Why are we required to pass in an object that's not going to be used as is? Just like in a previous discussion about the implications of injecting IEnumerable<T>, if you're injecting an abstraction, and the constructor then starts querying, modifying, examining, or otherwise fidget with the injected object, then you're probably injecting the wrong object.

Keep in mind that Constructor Injection is a declaration of requirements. It's the declaring class that advertises to the world: these are (some of) my invariants. If the class can't use the injected dependency as is, it suggests that it requested the wrong object from its clients. The class with the Injection Constructor should declare what it really needs. In this case, it needs to react to events.

In the next post, I'll show you a better (i.e. simpler) design for reacting to events.

Unsubscription (Update, September 9, 2013, 11:48 UTC) #

Some readers have commented that NeedyClass must keep the reference to IDependency in order to unsubscribe from the event. This is true, and something I overlooked when I originally banged together the sample code yesterday evening. Apparently, three unit tests were at least a test too few... :$

From a perspective of basic correctness, NeedyClass works appropriately, but as Geert van Horrik and Claus Nielsen point out, this could easily lead to resource leaks (although in practice, that depends on the effective lifetime of NeedyClass).

What happens when we start taking resource management into account?

Well, NeedyClass must be sure to unsubscribe from the event when it goes out of scope. The most correct way of making sure this happens is to implement IDisposable:

public class NeedyClass : IDisposable
{
    private readonly IDependency dependency;
 
    public NeedyClass(IDependency dependency)
    {
        if (dependency == null)
            throw new ArgumentNullException("dependency");
 
        this.dependency = dependency;
        this.dependency.ItHappened += this.OnItHappened;
    }
 
    private void OnItHappened(object sender, EventArgs e)
    {
        // Handle event here
    }
 
    public void Dispose()
    {
        this.dependency.ItHappened -= this.OnItHappened;
    }
}

While this weakens the argument that NeedyClass is requesting the wrong kind of dependency, this is a lot of work just to consume an event. Furthermore, it isn't particularly safe, because you'll have to remember to dispose of all of your NeedyClass instances.

Thus, a better approach is still warranted.

Comments

• ← Previous
• Archive
• Next →