Your DI framework is killing your code

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.

25 thoughts on “Your DI framework is killing your code

  1. infinitary

    Another reason for the noun-verbers is that adding domain logic to the value object will immediately become a design smell when the second feature/concern gets added. In large (aka real-world) systems there will be many various functionalities dealing with a given value object, and piling all those (often orthogonal) concerns on top of a given value object is clear violation of the single responsibility principle. This approach would terminate in gigantic balls of omniscient monster objects with tons of dependencies.

    I guess, I just wanted to say, this is hard, and not without a reason.

    PS: at one point I experimented with pure srp subclassing function wrappers around the value objects that were created on demand, but did not take it far enough to share it.

    PPS: I do have an aversion against any non-domain dependencies, such as mailer service, in domain objects, and this always led to a thin instrumenting layer of controllers around my empowered value objects.

    1. Yeah for sure there’s a balance. I think on one extreme you have all plain value objects and static methods; on the other extreme you can tend towards god classes with multiple responsibilities. In the real world, I often find the second feature encourages you to think about modelling your domain better. Sure you could slam it all in one class, or you could split everything into stateless static functions. Or the third, harder way is to come up with three new classes that represent the domain better. Domain modelling is hard, I think DI is often used an excuse to not do it.

      We should be less scared of having two or three closely related features touching a single class, if they are well coupled and don’t obviously violate the SRP. It’s just often people misinterpret SRP to mean the class should do one thing: it doesn’t, that encourages noun-verbers; SRP means the class should have a single reason to change. E.g. if I have three features closely related to Customer all touching the Customer class, then a change in what details are important about a Customer has a single place of change – even if that ultimately effects reporting, administration, etc. But the judgement in which details are closely related enough and which are separate is a tough one.

      The SRP subclassing wrappers sounds interesting, I think I can see where that would end up and I’m not sure I’d like it – but would be interesting to see it play out.

      I actually think the only valid dependencies your DI framework should inject are the non-domain dependencies: the things outside the current system, anyway – databases, emailers, web services. Everything else is internal to your domain and really ought to be modelled as such.

  2. Hi David very nice article.

    I got fed up with dependency injection a couple of years ago, specifically because I felt so “owned” by them. Like you say, injecting dependencies into objects created by your DB or HTTP libraries or frameworks is variously difficult or impossible. And at some point you want to put some auth code inside the MVC pipeline, at which point it’s not clear if your DI framework has even started yet. I’ve spent hours trying to understand how the DI framework interacts with MVC or Hibernate, and for a style of code that I’m frankly not in love with anymore.

    Compound this with having done a lot of Rails and Node projects in the last few years, where there’s almost no culture or need for DI frameworks at all. Save for Angular of course, I don’t think Angular benefits from having DI. Makes me wonder that DI is really just a peculiarity of .Net and Java projects. Outside of the “enterprise” style, nobody really cares for them.

    I’ve also come to the realization that testing my server side should be done on a more macro level, testing real HTTP through to real DB, so the need for super fine-grained TDD across all my classes has diminished dramatically.

    Then you realise that the only thing DI frameworks do well is lifetime management. Starting at DB session and closing it. Just the DB usually, that’s only one object you need to manage. Yet for some reason we end up injecting _everything_ and end up in the “don’t call us, we’ll call you” relationship with the DI. Owned.

    So I used a service locator. Just for the DB connection of course. The locator itself is a Func so it has a chance to be overloaded in tests, and I ended up using per HTTP request storage to store DB connections. The result being that there’s only a handful of places that ask for the DB, I understand how a few lines of code in my MVC pipeline work, instead of not understanding how Ninject or StructureMap work around the edges.

    Service locator sound too much like a global? Yes of course it is, but as you explain above, all your DI objects are effectively globals anyway. At least with a service locator it’s plainly obvious that they’re global and you know how to refactor your way out of it. With DI you’re back to staring at your config for hours, wondering how on earth you’re supposed to allow a second DB connection. How often do you need a second DB connection anyway?

    Controversial? Yeah definitely. Is my code better? Yes, because as you found out, I was finally able to move responsibly around my classes until it felt right. And I wasn’t spending hours in that “framework twilight zone” wondering what to do.

    1. Hey Tim – I think that’s a really important point: DI frameworks make moving responsibilities around difficult. They provide friction to refactoring, which means designs never iterate towards improvement. As you say, you’re owned by your DI framework.

      I think it is curious that DI exists only in the C# and Java world, where its abuse has spread like a cancer. Now people might argue that only on enterprise scale software projects – which are almost always in a safe language like C# or Java – do you need DI. But I’ve seen it on too many projects that definitely weren’t at enterprise scale. And there’s a strong counter argument that other languages don’t introduce the weight of code which DI is making it easier to manage.

      Service locator is actually a damned fine pattern. Given the choice of a DI framework and a hand-rolled service locator I’ve opted for the latter. As you say, it keeps you in complete control and is trivial to understand, it stops the magic that a DI framework introduces. Another crucial point is it provides friction – the trouble with DI frameworks is they make adding new dependencies so easy that everything might as well be a dependency. Because a service locator pattern introduces higher friction, you’re unlikely to expose every class via your service locator – you stick to just the obvious external dependencies like databases, web services etc.

  3. Vadim Shashenko

    First, I think that the design you are blaming here is actually better and even more natural. I see that your approach is how OOP is taught by some books about OO programming languages, but I think that approach often mixes up the responsibilities of different classes and creates bad code dependencies.

    Take the Customer class. It’s perfectly fine to have a public property Email in it because it’s a real attribute of a customer, as it is in real life. After your refactoring it became a private property. Now Customer has only the CreateReport method. Looks like a weird customer to me — he can only create reports. If something can only create reports, it should be called a customer. Then, creating a report can be a fairly large amount of code, which certainly will require dependencies to another libraries. If the report is an Excel report, for example, then the Customer class will have a dependency to some Excel library. I wouldn’t like that dependency in my code because it makes the Customer class bring unwanted libraries to some other parts of the code which uses the Customer class but not its reporting feature.

    So I would prefer to have a Customer with Name, Email, etc attributes. And then a method ReportingService.CreateReport(Customer customer) which has dependencies, which are specific for creating reports in particular format.
    This way it also makes it easy to understand that the reporting service can use only public (natural) attributes of the customer, not its hidden implementation details.

    Take the Email class. Having Email.Send() seems very bad idea to me. If we think in terms of real life objects, an email doesn’t send itself, it is done by a delivery service. An Email is a subject of action in this case. It should have only methods to manipulate its natural attributes: address fields, email body, attachments, etc. Again, having Email.Send() method will certainly require references into networking libraries for sending the email.

    So I would prefer an Email with To, CC, Body, Attachments attributes. And then a PostalService.Send(Email email).

    Those classes, Customer and Email, can be very small if you don’t have many attributes or logic inside them, but that’s ok and that’s natural — it reflects your current needs in the business domain. And when you need to add more logic to them, it will fit naturally without making them bloat. A comprehensive Email class can grow fairly big only with its own logic (multiple attachments, text formatting).

    Second, I think that DI is not the cause of the “problem” that you describe. DI affects the code only in terms of having extracted interfaces for every class, or just reference classes directly.

    1. It’s very subjective – it would massively depend on the actual domain. Unfortunately for this example, in most domains Customer would have many concerns and trying to bundle all the functionality related to customers would end up with a god class – which is absolutely not what I’m advocating.

      Taking report creation, as you say its highly likely that report creation would be complex – so should probably be represented as a family of related classes. Maybe there’s an ExcelCustomerReport, that is created by CustomerReport that is turn created by Customer. The ExcelCustomerReport maybe even has classes for worksheets or sections or cells or… Just because we have behaviour on stateful classes doesn’t mean all the behaviour goes on one class.

  4. I agree with you to a certain degree. I went with DI on a previous project which was handling dependencies in all my layers. I ended up regretting it.

    I do however think that simple DI helps with automatically injecting dependencies into my MVC controllers. The controllers get automatically instantiated in .Net so you have already lost control here. A simply configured DI container means I can deal with automatically injecting the correct dependencies. If you keep it simple and use the strongly typed methods then I think it can be useful.

    1. Thanks – I think this is the tough balance: DI is a very powerful technique and used appropriately massively improves designs; but all too easily it can be abused and then it is very hard to unpick when you have millions of dependencies injected left, right and centre.

      1. Christopher

        I’ve been using DI for more than 10 years and I’ve never seen more than 10 injections in a class or more than 100 service classes in a stand-alone service. I used SOA and Microservices to make sure no system ever grows to a size where more than a couple of engineers are needed to maintain it. I don’t see how “millions” of dependencies can be injected.

  5. I totally disagree! I am a big fan of DI and the criticized non-OO approach.

    1. DI frameworks are not evil at all. They are not killing code or anyting. You need to learn how to use it / how not to over-use it. There are other reasons why the OO approach is less common, see below.
    2. Once you build a more complex system, you will see that your value objects grow large with the OO approach. It breaks single responsibility and other clean code principles. Makes the code hard to read, maintain, etc.
    3. Once you need to pass your DTOs through many layers or even systems, you really want to have them as POJOs without any action methods. For example, a DB-related method have nothing to do in the UI layer.

  6. See, what you are suggesting is actually a strong violation of the single responsibility principle as well as separation of concerns. This is exactly like the code I used to write. While it really is easier to manage when it’s small, it’s a naive approach that doesn’t scale well.

    In a real life example of what you are suggesting, our User and Building classes for instance ended up over 10,000 lines long, doing things even just tangentially related to their parents. The real kickers were the cross business object ones who required more than one. Is it on Class X or Class Y? Sometimes it would end up on both accidentally causing the angels to cry.

    About two years ago we started a massive rewrite, and now we have lots of small consice objects that perform very specific tasks. They’re logical, organized and dependency injected. I would never wish the reverse on anyone.

    1. Yeah god classes are a danger – it always requires a balance. As you found, any class that’s 10k lines long is a massive debt, whatever approach led you there it was a mistake.

      But I’ve seen the opposite where you have tens of thousands of 10 line classes and it becomes impossible to get an overview of the system because there is no domain model. Then a change in one value object affects dozens of classes that interact with it.

  7. Paul

    Good luck with that. The reason anemic domains came about is because people hated the concept of an opaque and hard to debug god class – which is what it typically evolves into. Simple objects and “noun-verbers” compose much better and respond much better to change imo.

  8. Frank

    “By moving the method from ReportBuilder onto Customer, it is right next to the data it operates on”

    – What if the report assembles multiple different domain models?

    “A Customer can CreateReport(), a Report can RenderAsEmail(), and an Email can Send().”

    – A ReportBuilder takes a Customer (and probably other domain models) and creates a report, a ReportRenderer renders a report as an email (or pdf etc.), an EmailTransport sends an EmailMessage …

    “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.”

    – Then give functionality to the customer, but not for the sake of having one.

    And as David Green said your design considerations clearly lead to god objects.

  9. Christopher

    I don’t think you completely understand the DI frameworks. Just because a service in a DI framework is often a stateless singleton doesn’t mean they don’t allow stateful injection: EntityManager for JPA is a great example. Sure, there are other services with absolutely no state, but don’t blame the frameworks for promoting statelessness, it’s a practice promoted by many other software designs e.g. REST. I believe I have enough experience to say that state replications in a clustered environment does not scale and is not worth the pain.

    Order.SubmitForPicking() sounds great on paper, that’s until you realize in order to submit an order for picking, you’ll need other value objects such as Customer, Account, Warehouse, Picker, Robot, Supervisor, Audit information …. the list goes on and on. How are you going to inject these info into an Order? And do they really belong in an order? I certainly don’t want to see all of these info in my JSON when I retrieve a single order on the client side.

    The moment you move behaviors close to a value object is also the moment a plain object is no longer a plain object. Furthermore, a behavior laden value object becomes hard to evolve, extend and modify. There’s no clear boundary of where data ends and behavior begins, you certainly cannot package them up as a library and give it to your clients, you’ll have to create a separate set of value objects for that and start a maintenance nightmare.

    1. A stateless service could still have domain objects with behaviour – they are created and destroyed within the bounds of a single request, nothing forces you to start serializing complex objects across architectural boundaries.

      You’re right, if SubmitForPicking needs all of those objects then there are some more responsibilities – some more classes, more domain objects – that should be extracted, I wouldn’t want to see a single method with interactions with seven other classes.

  10. Friends don’t let Friends Inject!

    By injecting you very quickly lose the ability to reason about your code in quick and pure way.

    I think David did make the point it is about balance, The ‘God Object’ at one end and ‘noun-verbers’ at the other end.

    I personally value the ability refactor and shuffle methods and function around more then anything… Why because I never get it right the first time and that is after 20+ years of experience.

    I noticed a few of the replies and and in article it self the lovely ‘C’ word “coupling” was brought up, but it partner cohension was not. There appears to be total lack of clear thinking around classes and how to achieve the balance.

    So here is my perspective.

    – Classes as name spaces, where logically comparable behaviours are housed.
    – One wants to look at a classes within the context of all the classes in the application and discover comparable levels of abstraction. So in the Customer example within the context of the application the report method should be in the customer class. It cohensive from the perspective of the abstraction level.

    But if the application was primary some kind of generic reporting engine, then maybe it would be the right place for the report method.

    – The key I would like to make is this, applying this type of thinking does not lead to god objects. It is the lack of skill in the implementation.

    Lets say that every report produced by our customer system required same heading and and special audit footer that provided a count as to how many time the report had been printed and by whom it was printed.

    This is the strength of the functional world, depending on the language you have access one could have the implementation in external functions that are mixed in like traits.

    All of sudden looking at the customer object can see a nice encapsulated customer with a range of behaviours which a customer has. The implementation of those behaviours are decoupled. They could reside in top level functions that support the implementation across a number of value object classes.

    So what if the customer has hundreds of behaviours… That is ok it is called modelling, you are either going to adopt inheritance or composition depending on the nature of customer.

    All of this can be and regularly is done without DI even with million plus lines of code. The complaints are all emotionally based, yes you need to type more to hand in the dependencies (Which are not that many if modelled correctly) and what do think autocomplete is for anyway.

    Bottom line friends don’t let friends inject, it take away code flexibility and everyones ability to reason and understand the what is happening in the code. Even with Google Guice, most people are going to get inject addiction. Which leads to every class depending on every other class because they all injected and people start pull stuff from everywhere.

    1. “Friends don’t let friends inject” – haha, awesome, thank you!!

      I think it is exactly this conflict between cohesion and coupling that I’m trying to highlight. God objects are the ultimate in excessive coupling, whereas noun-verbers are the ultimate in low cohesion. It is within this tension that good designs are found.

      I also like the idea of a class as a namespace – it is, in one sense, nothing more than a way of grouping related functionality. It is, as you say, about comparable levels of abstraction. A customer class with too many responsibilities probably exists at too many levels of abstraction. But a well modelled customer will have a sensible number of responsibilities, for a given level of abstraction, for the domain it exists within.

  11. Sorry maybe I miss something but how would you create different type of reports?

    With DI (or even without DI, it actually doesn’t matter) I would do: (pure DTO object serializable/jsonable)

    ICustomerReporter with createReport(Customer) and then impls:


    And yes, if using DI I would wire correct impl in the spring (for example – I like xml becuase you can just take 3d party .jar and wire it with single file without actually needing write bit of code and deploy/run directly), without DI I would write another class IReporterFactory with getReporter method.

    Would you be adding those different implementations in Customer class (or inherit Customer and override it’s report method)? Imagine you have 15 different reporters…

  12. DI can certainly make developers blind to issues due to its shininess but I certainly wouldn’t say it’s bad. I certainly could never recommend using a service locator instead.

    I’d probably be bad and use a “noun-verber” to send the email, but I could see it being an email method on the client instead of the report, then the report could implement an IEmailMessage interface or be converted to email somehow (ConvertToEmail could be on the report, but then do you really want to have a method per filetype you support?).

Leave a Reply

Fill in your details below or click an icon to log in: Logo

You are commenting using your account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s