Dealing with the “Too many dependencies” problem

Earlier I asked whether people thought the issue of passing in too many dependencies into a class was an actual problem. This was in response to another post I did about test smells. A few people had asked me how the problem of a controller having way too many dependencies should be solved. My response to them was to apply Single Responsibility Principle, which is pretty much what others also said.

The problem

Here is the problem:

In this case it is a controller, but it can well be any other type of class.

Dependency Injection does not solve tight coupling

Why is this a problem? After all, we are applying Dependency Injection.

We are, but we are not decoupling classes. All we are doing is allowing a specific implementation to be swapped out easily. Whether we’re doing it because we want to change behaviors in tests or at runtime, all we’re doing is avoiding depending on a specific implementation.

What we are not doing is automagically making our code clean and sustainable.

The more dependencies our class takes in, the higher its efferent coupling to other classes, meaning that it knows a little bit too much. If it knows too much, it is  most likely because it is also doing too much. The class is no longer focused on one thing, and as a consequence we are creating somewhat brittle software that is too tightly coupled, even if we are passing in our dependencies, even if we are not using new anymore.

How did we get here?

We all agree that DI is good and so is Composition over Inheritance. And this causes us to end up with classes that act as coordinators or require information or perform actions on these different classes. Other times we end up here because of:

Framework impositions

Often, frameworks impose restrictions that take us down the wrong path. Such is the case of ASP.NET MVC when using the default routing conventions. We mostly encapsulate functionality under a specific controller because the routing suites us. In other words, from a user’s perspective it makes sense to have many things under the /checkout/{action} path, even if that leads us to bloated controllers that take many dependencies which have little to do with each other.

Badly named classes

XYZService or XYZManager named classes are yet another example of ending up with too many dependencies. When we start to encapsulate everything under a service or manager, we find that despite some of the dependencies only being used by one or two methods, still have to be passed in.

IoC Containers

As developers we no longer worry about wiring up all the different required dependencies as Containers now do this for us. This allows us to inject and inject and inject.

Helping solve the problem

First and foremost, and despite it being beaten to death, we need to apply Single Responsibility Principle. I’ve written in the past techniques that have often helped me accomplish this. Taking this into account, there’s also a series of Do’s and Don’ts that can help:

What you can do

  • Work around framework limitations. For instance, in the case of ASP.NET MVC, you can override the routing behavior to not tie you down to using one class just because you need to keep the same URL. This can be done in various ways. Google Controllerless Actions for some examples (just a starting point).
  • Create small focused classes. Why create a CustomerServices that takes in five dependencies as opposed to creating MakeCustomerPreferredCommand and InvalidateCustomerCommand class that takes one dependency and has a single method to do what is required?
  • Extract infrastructure. Don’t pass in dependencies that have to do with infrastructure, specially if these are repetitive. Use techniques such as AOP to apply these at the level where they belong.
  • Think about Patterns. Often patterns can help solve the problem of too many dependencies. For instance, many times, what’s deemed as a large collaborative class can instead be turned into a series of classes using the Chain of Responsibility pattern.
  • Events. Event driven architecture is a great way to provide higher decoupling and independence between objects.
  • CQRS. Well I just had to mention this one because it’s fashionable. In all seriousness though, this also allows for lower of dependencies since it separates responsibilities into smaller commands and separate queries, which again is our goal.

What you shouldn’t do

  • You shouldn’t switch to using Service Locator as opposed to Dependency Injection. All you’re doing is hiding the problem. Dependency Injection is actually beneficial in that it is showing us that we are breaking SRP and have a high coupling.
  • You shouldn’t stop using an IoC Container. There are other ways to prevent the  Inject Happy Developer. Testing your code is one of them. As you write tests, you’ll suffer the pain of having to set things up over and over again (and refrain from encapsulating that).

These are just some simple guidelines. They are not set in stone.

Injection Happy Detector

Finally, while writing the initial post, I came up with an idea of writing a ReSharper plugin that could help detect classes that have too many dependencies. The result is
the InjectionHappy Detector. Once installed, it gives you a warning if it finds a constructor taking in too many interfaces. This number by default is 3 (just some random value, you can change it yourself under Options). It’s a very simple plugin and not full-proof. More of an experiment. You’re free to fork it and enhance it anyway you wish.

17 thoughts on “Dealing with the “Too many dependencies” problem

  1. Alexander Beletsky (@alexbeletsky)

    Thanks, good post.

    But it is does not give me clear answer of what to do. I clearly see SRP and patterns and to be honest for long period of time did not realize why people are having that problem at all, before I seen it on actual project and was not able to fix that.

    Suppose I still working with that Checkout controller, it has a lot of dependencies, but I’m sure – all of that is required. So, none could be just thrown away. Idea with commands is good, but still for every object I create inside the controllers action I need to have the dependency.. Where the dependency come from? Of cause from controllers constructor. So, I still need to inject yet another interface into controller.

    public ActionResult SomeAction(…)
    {
    var createUser = new CreateUserCommand(_userRepository);
    var copyInvoice = new CopyInvoiceCommand(_draftInvoicesRepository, _invoicesRepository, _userSettingsRepository);
    var bookInvoice = new BookInvoiceCommand(_invoicesRepository, _bookingServices);

    // …
    }

    So, the constructor have to be:

    SomeController(IUserRepository users, IDraftInvoicesRepository drafts, InvoicesRepository invoices, IUserSettingsRespository settings, IBookingService service)

    Keeping the Controllers small (meaning do only one thing) is also bad, since we want a kind of ‘atomic’ operations. Like, when I create user I need: check for existance, create new record, call service to send email, redirect somewhere etc. So, for this simple case I need at least 3 dependencies – user repo, email service and redirect service (example as rather simple, but it shows the case). Splitting it up to 3 controllers is waste, more over it will complicate a client a bit that requires no to accomplish 3 posts instead on 1.

    We did something like that – but instead of 1 controller with many dependencies you have several, including one that acts as facade and does WebRequest() POST/GET to next URL in chain. That worked, but.. I don’t think it great solution either.

    I came up to conclusion that it’s almost important to get rid of that ‘Big-constructor’ controllers. Even if I still treat it as smell, I think that only way you can do with ASP.NET MVC right now.

    Reply
    1. Alexander Beletsky (@alexbeletsky)

      >> I came up to conclusion that it’s almost important to get rid of that ‘Big-constructor’ controllers. Even if I still treat it as smell, I think that only way you can do with ASP.NET MVC right now.

      And here.. I wanted to say – ‘almost impossible’ …

      Reply
    2. hhariri Post author

      Much of the code in that controller does not belong in the controller. Some belong at infrastructure level, some at other levels outside of the domain of the controller. As soon as you start splitting that up into smaller units, automatically the number of dependencies are reduced.

      There is the option of Controllerless actions and I personally see nothing wrong with it, however as you mention, a class also has cohesion so there’s nothing prohibiting it having more than one action where these things are related, and again, the dependencies required are a good indication of the cohesion the class has.

      As for a redirect service, why?

      Reply
      1. Alexander Beletsky (@alexbeletsky)

        Maybe I did a little to much showing how many dependencies I need, sure for redirect it’s just enough to return RedirectResult.

        Anyways, I would be happy to see next follow-up post about using Controller-less actions for getting rid of ‘fat-constructors’.. Will you? 🙂

    3. Steve Smith (@ardalis)

      Alexander,
      Remember your Controller is a UI concept. It shouldn’t be doing a ton of work that isn’t directly related to choosing a View and providing that View with the data it needs to display. If it needs to create a user, send a notification, etc, etc, that’s not its job. That should all go into a service, which the Controller can call (and which can be injected via the constructor). This will make your controller much smaller, and you can separate out tests for how your constructor behaves (e.g. given an error, show this other view) with tests for how your service should behave.

      Reply
  2. Lars-Erik Kindblad (@kindblad)

    Interesting post. I totally agree with you about having small focused command classes with one method. This makes the code easy to test, easy to maintain and it solves the problem with too many dependencies.
    However frontend classes such as MVC Controllers and MVVM ViewModels should be simple facade classes, where each method has one line of code – a call to a command class.
    For facade classes a Service Locator is the proper thing to use since you dont want to create dependencies until they are actually needed. For instance with MVC only one action is called per request, thus its a waste to create all the dependencies needed by the other actions on the controller when they won’t be called, and that’s what happens if dependency injection is used. For command classes dependency injection is the proper thing to use.

    Reply
  3. Pingback: Last Post - Hadi Hariri - Devlicio.us - Just the Tasty Bits

  4. schotime

    I have faced this issue in a few projects now, and I always come back to going with once class per action. I have also just recently formalised into a class library on github.

    It can be found here: http://github.com/schotime/ActionControllers

    Its only in its early stages but if you like the idea, fork it or create a feature request.

    Reply
  5. saintedlama

    When taking a look at the wonderful service staccato an option to tidy up that dependency mess is to use events for loose coupling services. For example the IEmailerService and the IIncentiveService are perfect candidates for event communication.

    IMHO the suggested Controllerless Actions approach does not really fight the problem’s root but eases the effects.

    Reply
      1. saintedlama

        A generic event emitter component could be injected in the controller then the controller would use this event emitter component to fire events such as RegistrationFinished, OrderProcessed. The IIncentiveService would for example listen on the RegistrationFinished event and use this event data to check if an incentive should be given and give that incentive to the user. So the Controller does not need a direct reference on the IIncentiveService but fires events. This decouples IncentiveService from controller.

        Sample Code:

        public class CheckoutController : StoreController
        {
        public CheckoutController(EventEmitter eventEmitter, ..)
        {
        this.eventEmitter = eventEmitter;
        }

        public ViewResult Checkout(CheckoutViewModel model)
        {
        // … ASP.NET MVC boiler plate validation

        // … Do checkout logic

        eventEmitter.Fire(new EventData(…));
        }
        }

        IIncentiveService and IEmailService would probably listen on this event and give an incentive and send a registration successful message to the user.

        The tricky job is to design the events/event data objects.

    1. schotime

      In this situation you might just embed the commandInvoker in the baseController so that you don’t keep having to pull it in. I also use the InputModel (always a class) as the way to find the handler that handles this type of data/action.

      Reply
  6. Steve Smith (@ardalis)

    One obvious problem with the controller shown here is with the level of abstraction it’s using. Instead of a dozen low-level interfaces, it could have a single ICheckoutService interface passed into it. Might that service then have this problem? Perhaps, but most likely it, too, could benefit from using just a few higher-level interfaces rather than a dozen low-level ones. Clean Code recommends one level of abstraction per function, and includes smell G6: Code at Wrong Level of Abstraction, that relate to this as well.

    Reply

Leave a Reply

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

WordPress.com Logo

You are commenting using your WordPress.com 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