To constructor or to property dependency?

Krzysztof Koźmic — Wed, 05 Sep 2012 12:32:00 GMT

Us, developers, are a bit like that comic strip (from always great xkcd):

We can endlessly debate over tabs versus spaces (don't even get me started), whether to use optional semicolon or not, and other seemingly irrelevant topics. We can have heated, informed debates with a lot of merit, or (much more often) not very constructive exchanges of opinions.

I mention that to explicitly point out, while this post might be perceived as one of such discussions, the goal of it is not to be a flamewar bait, but to share some experience.

So having said that, let's cut to the chase.

Dependencies, mechanics and semantics

Writing software in an object oriented language, like C#, following established practices (SOLID etc) we tend to end up with many types, concentrated on doing one thing, and delegating the rest to others.

Let's take an artificial example of a class that is supposed to handle scenario where a user moved and we need to update their address.

public class UserService: IUserService
{
   // some other members

   public void UserMoved(UserMovedCommand command)
   {
      var user = session.Get<User>(command.UserId);

      logger.Info("Updating address for user {0} from {1} to {2}", user.Id, user.Address, command.Address);

      user.UpdateAddress(command.Address);

      bus.Publish(new UserAddressUpdated(user.Id, user.Address));
   }
}

There are four lines of code in this method, and three different dependencies are in use: session, logger and bus. As an author of this code, you have a few options of supplying those dependencies, two of which that we're going to concentrate on (and by far the most popular) are constructor dependencies and property dependencies.

Traditional school of thought

Traditional approach to that problem among C# developers goes something like this:

Use constructor for required dependencies and properties for optional dependencies.

This option is by far the most popular and I used to follow it myself for a long time. Recently however, at first unconsciously, I moved away from it.

While in theory it's a neat, arbitrary rule that's easy to follow, in practice I found it is based on a flawed premise. The premise is this:

By looking at the class' constructor and properties you will be able to easily see the minimal set of required dependencies (those that go into the constructor) and optional set that can be supplied, but the class doesn't require them (those are exposed as properties).

Following the rule we might build our class like that:

public class UserService: IUserService
{
   // some other members

   public UserService(ISession session, IBus bus)
   {
      //the obvious
   }

   public ILogger Logger {get; set;}
}

This assumes that session and bus are required and logger is not (usually it would be initialised to some sort of NullLogger).

In practice I noticed a few things that make usefulness of this rule questionable:

It ignores constructor overloads. If I have another constructor that takes just session does it mean bus is an optional or mandatory dependency? Even without overloading, in C# 4 or later, I can default my constructor parameters to null. Does it make them required or mandatory?
It ignores the fact that in reality I very rarely, if ever, have really optional dependencies. Notice the first code sample assumes all of its dependencies, including logger, are not null. If it was truly optional, I should probably protect myself from NullReferenceExceptions, and in the process completely destroy readability of the method, allowing it to grow in size for no real benefit.
It ignores the fact that I will almost never construct instances of those classes myself, delegating this task to my IoC container. Most mature IoC containers are able to handle constructor overloads and defaulted constructor parameters, as well as making properties required, rendering the argument about required versus optional moot.

Another, more practical rule, is this:

Use constructor for not-changing dependencies and properties for ones that can change during object's lifetime.

In other words, session and bus would end up being private readonly fields — the C# compiler would enforce that once we set them (optionally validating them first) the fields in the constructor, we are guaranteed to be dealing with the same (correct, not null) objects ever after. On the other hand, Logger is up in the air, since technically at any point in time someone can swap it for a different instance, or set it to null. Therefore, what usually logically follows from there, is that property dependencies should be avoided and everything should go through the constructor.

I used to be the follower of this rule until quite recently, but then it does have its flaws as well.

It leads to some nasty code in subclasses where the base class has dependencies. One example I saw recently was a WPF view model base class with dependency on dispatcher. Because of that every single (and there were many) view model inheriting from it, needed to have a constructor declared that takes dispatcher and passes it up to the base constructor. Now imagine what happens when you find you need event aggregator in every view model. You will need to alter every single view model class you have to add that, and that's a refactoring ReSharper will not aid you with.
It trusts the compiler more than developers. Just because a setter is public, doesn't mean the developers will write code setting it to some random values all over the place. It is all a matter of conventions used in your particular team. On the team I'm currently working with the rule that everybody knows and follows is we do not use properties, even settable ones, to reassign dependencies, we're using methods for that. Therefore, based on the assumption that developers can be trusted, when reading the code and seeing a property I know it won't be used to change state, so readability and immediate understanding of the code does not suffer.

In reality, I don't actually think the current one, or few of my last projects had even a requirement to swap service dependencies. Generally you will direct your dependency graphs from more to less volatile objects (that is object will depend on objects with equal or longer lifespan). In few cases where that's not the case the dependencies would be pulled from a factory, and used only within the scope of a single method, therefore not being available to the outside world. The only scenario where a long-lived dependency would be swapped that I can think of, is some sort of failover or circuit-breaker, but then again, that would be likely dealt with internally, inside the component.

So, looking back at the code I tend to write, all dependencies tend to be mandatory, and all of them tend to not change after the object has been created.

What then?

This robs the aforementioned approaches from their advertised benefits. As such I tend to draw the division along different lines. It does work for me quite well so far, and in my mind, offers a few benefits. The rule I follow these days can be explained as follows:

Use constructor for application-level dependencies and properties for infrastructure or integration dependencies.

In other words, the dependencies that are part of the essence of what the type is doing go into the constructor, and dependencies that are part of the "background" go via properties.

Using our example from before, with this approach the session would come via constructor. It is part of the essence of what the class is doing, being used to obtain the very user whose address information we want to update. Logging and publishing the information onto the bus are somewhat less essential to the task, being more of an infrastructure concerns, therefore bus and logger would be exposed as properties.

This approach clearly puts distinction between what's essential, business logic, and what is bookkeeping. Properties and fields have different naming conventions, and (especially with ReSharper's 'Color identifiers' option) with my code colouring scheme have significantly different colours. This makes is much easier to find what really is important in code.
Given the infrastructure level dependencies tend to be pushed up to base classes (like in the ViewModel example with Dispatcher and EventAggregator) the inheritors end up being leaner and more readable.

In closing

So there you have it. It may be less pure, but trades purity for readability and reduces amount of boilerplate code, and that's a tradeoff I am happy to make.

Krzysztof Kozmic : c#

To constructor or to property dependency?

Depen­den­cies, mechan­ics and semantics

Tra­di­tional school of thought

What then?

In clos­ing

Dependencies, mechanics and semantics

Traditional school of thought

In closing