Tag: ioc

Your DI framework is killing your code

On By David GreenIn Software development28 Comments

I read a really interesting post recently looking at the difference between typical OO code and a more functional style. There’s a lot to be said for the functional style of coding, even in OO languages like Java and C#. The biggest downside I find is always one of code organisation: OO gives you a discoverable way of organising large amounts of code. While in a functional style you might have less code, but it’s harder to organise it clearly.

It’s not Mike’s conclusion that I want to challenge, it’s his starting point: he starts out with what he describes as “typical object oriented C# code”. Unfortunately I think he’s bang on: even in this toy example, it is exactly like almost all so-called “object oriented” code I see in the wild. My issue with code like this is: it isn’t in the least bit object oriented. It’s procedural code haphazardly organised into classes. Just cos you’ve got classes, don’t make it OO.

What do I mean procedural code? The typical pattern I see is a codebase made up of two types of classes:

  1. Value objects, holding all the data with no business logic
    Extra credit here if it’s an object from your persistence layer, nhibernate or the like.
  2. Classes with one or two public functions –  they act on the value objects with no state of their own
    These are almost always “noun-verbers”

A noun-verber is the sure-fire smell of poor OO code: OrderProcessor, AccountManager, PriceCalculator. No, calling it an OrderService doesn’t make it any better. Its still a noun-verber you’re hiding by the meaningless word “service”. A noun-verber means its all function and no state, it’s acting on somebody else’s state. It’s almost certainly a sign of feature envy.

The other design smell with these noun-verbers is they’re almost always singletons. Oh you might not realise they’re singletons, because you’ve cleverly hidden that behind your dependency injection framework: but it’s still a singleton. If there’s no state on it, it might as well be a singleton. It’s a static method in all but name. Oh sure its more testable than if you’d actually used the word “static”. But it’s still a static method. If you’d not lied to yourself with your DI framework and written this as a static method, you’d recoil in horror. But because you’ve tarted it up and called it a “dependency”, you think it’s ok. Well it isn’t. It’s still crap code. What you’ve got is procedures, arbitrarily grouped into classes you laughably call “dependencies”. It sure as hell isn’t OO.

One of the properties of good OO design is that code that operates on data is located close to the data. The way the data is actually represented can be hidden behind behaviours. You focus on the behaviour of objects, not the representation of the data. This allows you to model the domain in terms of nouns that have behaviours. A good OO design should include classes that correspond to nouns in the domain, with behaviours that are verbs that act on those nouns: Order.SubmitForPicking(). UserAccount.UpdatePostalAddress(), Basket.CalculatePriceIncludingTaxes().

These are words that someone familiar with the domain but not software would still understand. Does your customer know what an OrderPriceStrategyFactory is for? No, then it’s not a real thing. Its some bullshit you made up.

The unloved value objects are, ironically, where the real answer is to the design problem. These are almost always actual nouns in the domain. They just lack any behaviours which would make them useful. Back to Mike’s example: he has a Customer class – its public interface is just an email address property, a classic value object: all state and no behaviour [if you want to play along at home, I’ve copied Mike’s code into a git repo; as well as my refactored version].

Customer sounds like a good noun in this domain. I bet the customer would know what a Customer was. If only there were some behaviours this domain object could have. What do Customers do in Mike’s world? Well, this example is all about creating and sending a report. A report is made for a single customer, so I think we could ask the Customer to create its own report. By moving the method from ReportBuilder onto Customer, it is right next to the data it operates on. Suddenly the public email property can be hidden – well this seems a useful change, if we change how we contact customers then only the customer needs to change, not also the ReportBuilder. It’s almost like a single change in the design should be contained within a single class. Maybe someone should write a principle about this single responsibility malarkey…

By following a pattern like this, moving methods from noun-verbers onto the nouns on which they operate, we end up with a clearer design. A Customer can CreateReport(), a Report can RenderAsEmail(), and an Email can Send(). In a domain like this, these seem like obvious nouns and obvious behaviours for these nouns to have. They are all meaningful outside of the made up world of software. The design models the domain and it should be clear how the domain model must change in response to each new requirement – since each will represent a change in our understanding of the domain.

So why is this pattern so uncommon? I blame the IoC frameworks. No seriously, they’re completely evil. The first thing I hit when refactoring Mike’s example, even using poor man’s DI, was that my domain objects needed dependencies. Because I’ve now moved the functionality to email a report from ReportingService onto the Report domain object, my domain object now needs to know about Emailer. In the original design it could be injected in, but with an object that’s new’d up, how can I inject the dependency? I either need to pass Emailer into my Report at construction time or on sending as email. When refactoring this I opted to pass in the dependency when it was used, but only because passing in at construction time is cumbersome without support.

Almost all DI frameworks make this a royal pain. If I want to inject dependencies into a class that also has state (like the details of the customer’s report), it’s basically impossible, so nobody does it. It’s better, simpler, quicker to just pull report creation onto a ReportBuilder and leave Customer alone. But it’s wrong. Customer deserves to have some functionality. He wants to be useful. He’s tired of just being a repository for values. If only there was a way of injecting dependencies into your nouns that wasn’t completely bullshit.

For example using NInject – pretty typical of DI frameworks – you can create a domain object that requires injected dependencies and state by string typing. Seriously? In the 21st century, you want me to abandon type safety, and put the names of parameters into strings. No. Just say no.

No wonder when faced with these compromises people settle for noun-verbers. The alternatives are absolutely horrific. The only DI framework I’ve seen which lets you do this properly is Guice‘s assisted injection. Everybody else, as far as I can see, is just plain wrong.

Is your DI framework killing your code? Almost certainly: if you’ve got value objects and noun-verbers, your design is rubbish. And it’s your DI framework’s fault for making a better design too hard.

Dependency injection with PostSharp

On By David GreenIn Software developmentLeave a comment

I don’t really like IoC containers. Or rather, I don’t like the crappy code people write when they’re given an IoC container. Before you know it you have NounVerbers everywhere, a million dependencies and no decent domain model. Dependencies should really be external to your application; everything outside of the core domain model that your application represents.

  • A web service? That’s a dependency
  • A database? Yup
  • A message queue? Definitely a dependency
  • A scheduler or thread pool? Yup
  • Any NounVerber (PriceCalculator, StockFetcher, BasketFactory, VatCalculator) no! Not a dependency. Stop it. They’re all part of your core business domain and are actually methods on a class. If you can’t write Price.Calculate() or Stock.Fetch() or new Basket() or Vat.Calculate() then fix your domain model first before you go hurting yourself with an IoC container

A while back I described a very simple, hand-rolled approach to dependency injection. But if we wave a little PostSharp magic we can improve on that basic idea. All the source code for this is available on github.

It works like this: if we have a dependency, say an AuthService, we declare an interface that business objects can implement to request that they have the dependency injected into them. In this case, IRequireAuthService.

class User : IRequireAuthService
{
  public IAuthService AuthService { set; private get; }

We create a DependencyInjector that can set these properties:

public void InjectDependencies(object instance)
{
  if (instance is IRequireAuthService)
    ((IRequireAuthService)instance).AuthService = AuthService;
    ...
}

This might not be the prettiest method – you’ll end up with an if…is IRequire… line for each dependency you can inject. But this provides a certain amount of friction. While it is easy to add new dependencies, developers are discouraged from doing it. This small amount of friction massively limits the unchecked growth of dependencies so prevalent with IoC containers. This friction is why I prefer the hand-rolled approach to off-the-shelf IoC containers.

So how do we trigger the dependency injector to do what it has to do? This is where some PostSharp magic comes in. We declare an attribute to use on the constructor:

  [InjectDependencies]
  public User(string id)

Via the magic of PostSharp aspect weaving this attribute causes some code to be executed before the constructor. This attribute is simply defined as:

public sealed override void OnEntry(MethodExecutionArgs args)
{
  DependencyInjector.CurrentInjector.InjectDependencies(args.Instance);
  base.OnEntry(args);
}

And that’s it – PostSharp weaves this method before each constructor with the [InjectDependencies] attribute. We get the current dependency injector and pass in the object instance (i.e. the newly created User instance) to have dependencies injected into it. Just like that we have a very simple dependency injector. Even better all this aspect weaving magic is available with the express (free!) edition of PostSharp.

Taking it Further

There are a couple of obvious extensions to this. You can create a TestDependencyInjector so that your unit tests can provide their own (mock) implementations of dependencies. This can also include standard (stub) implementations of some dependencies. E.g. a dependency that manages cross-thread scheduling can be replaced by an immediate (synchronous) implementation for unit tests to ensure that unit tests are single-threaded and repeatable.

Secondly, the DependencyInjector uses a ThreadLocal to store the current dependency injector. If you use background threads and want dependency injection to work there, you need a way of pushing the dependency injector onto the background thread. This generally means wrapping thread spawning code (which will itself be a dependency). You’ll want to wrap any threading code anyway to make it unit-testable.

Compile Time Checks

Finally, the most common failure mode we encountered with this was people forgetting to put [InjectDependencies] on the constructor. This means you get nulls at runtime, instead of dependencies. With a bit more PostSharp magic (this brand of magic requires the paid-for version, though) we can stop that, too. First, we change each IRequire to use a new attribute that indicates it manages injection of a dependency:

[Dependency]
public interface IRequireAuthService
{
  IAuthService AuthService { set; }
}

We configure this attribute to be inherited to all implementation classes – so all business objects that require auth service get the behaviour – then we define a compile time check to verify that the constructors have [InjectDependencies] defined:

public override bool CompileTimeValidate(System.Reflection.MethodBase method)
{
  if (!method.CustomAttributes.Any(a => a.AttributeType == typeof(InjectDependenciesAttribute)))
  {
    Message.Write(SeverityType.Error, "InjectDependences", "No [InjectDependencies] declared on " + method.DeclaringType.FullName + "." + method.Name, method);
    return false;
  }
  return base.CompileTimeValidate(method);
}

This compile time check now makes the build fail if I ever declare a class IRequireAuthService without adding [InjectDependencies] onto the class’ constructor.

Simple, hand-rolled dependency injection with compile time validation thanks to PostSharp!