Service locators don’t belong in controllers

« »

It’s a pretty common pattern: instead of listing all the dependencies, you simply pass in the service locator, and look up the dependencies you need at runtime. Many frameworks (coughSymfonycough) advocate this model, but it has some serious drawbacks. So many, in fact, that I have a general rule of thumb for controllers:

Service locators don’t belong inside controllers. Period.

Injecting a service locator into a controller completely breaks the paradigms of object-oriented application design that we are trying to foster when we write our applications. For starters, it gives us cart blanche to bloat the controllers into gigantic logic centers. It also means that we stop understanding what dependencies our controllers are using, because we’re no longer responsible for actually defining them. And the worst one of all, is it flouts the principles of Dependency Inversion and Separation of Concerns.

That’s not to say that the service locator doesn’t belong inside an object ever. In fact, I use a service locator/dependency inversion container for locating and creating controllers inside of the front controller before dispatching them. This is a perfectly normal use case, and one that’s perfectly acceptable. In this case, the service locator is more of a factory than a master record of all objects.

But if you’re including it inside your controllers, what you’re going to end up with is a situation where the controllers are bloated, contain too much logic (which should be delegated to the model anyway), and are completely untestable. You’ll be unable to identify the dependencies, because you’ll always be able to introduce new ones, and you’ll probably end up violating other principles of good object-oriented design.

Instead, design controllers that can stand alone, list their dependencies at construction time, and that do a small, discrete job. Your applications will be testable, extensible, and best of all, correct.

Brandon Savage is the author of Mastering Object Oriented PHP and Practical Design Patterns in PHP

Posted on 7/20/2015 at 12:49 pm
Categories: PHP

Douglas Greenshields (@bedroomation) wrote at 7/20/2015 3:34 pm:

I’m going to go ahead and strongly disagree with this.

The term “controller” I don’t think meaningfully refers to the class, or object – rather, it refers to the public methods of the class that take a request and return a response. Therefore, they are bundles of functions that are actually independent of each other. Having a constructor method, therefore, that allows injection of services that any of the controller methods in the class may wish to use creates an unnecessary coupling between these methods. And for what gain? You can fetch your services out of a service locator at the beginning of the controller method if you want to be explicit about the services that you are depending on. The methods can be moved independently then without having to unpick any plumbing.

Of course it’s bad to refer to too many external services within one controller function – they should be kept as simple and thin as possible with as few external dependencies as possible – but this principle is orthogonal to whether or not you inject a service locator.

In short, I would say that controllers are functions, not objects. This advice seems to be trying to apply general OOP principles to a situation where they simply don’t apply.

Patrick Dos Santos wrote at 7/20/2015 5:30 pm:

I agree with Douglas here.

A controller is a function. Services should be injected for each function, and not for the whole class.

By the way, Symfony encourages controllers defined as services (http://symfony.com/doc/current/cookbook/controller/service.html) and solves the service per controller (as in function) problem

Guido Contreras Woda (@guiwoda) wrote at 7/20/2015 6:08 pm:

@bedroomation I have to disagree with what you’re arguing, but because it seems too based on the actual misuse of the Controller pattern.

If you feel that Controller methods should be separated, then you should separate them. If multiple methods in the same Controller class means you get bloated, useless dependencies, then you should consider separating them to make them coherent again.

Controller cohesion is very fragile. I’ve seen Controllers designed based on very general concepts (UserController, ProductController) and end up having lots of methods that covered multiple, unrelated parts of the application. Instead, focus your Controllers on a set of coherent endpoints and create new ones when they get too bloated.

Also, what Brandon advocates here is also useful for maintenance:
– Being forced to share a constructor throughout your methods will hint when a Controller class is taking too many responsibilities, and that will help you refactor into a better model.
– Having a single DI point will help identify dependencies easier and help build a more solid architecture. Having to search each method for its dependencies in order to identify possible architectural breaches could make for a more complex scenario.

Service Locators and DI containers usually make for more opaque dependencies and can (and will) cause more trouble than good if overused. If you want to have a clean, controlled design with a predictable architecture, you should try your best to avoid tools that let you access almost any service whatsoever in any point of your app. This kind of restrictions will pay off as the application grows.

Evaldo Junior (@InFog9) wrote at 7/21/2015 3:42 am:

Very good explanation. I have this issue with Symfony (not only Symfony) and untestable controllers due to heavy usage of the dependency injection container. Explaining to people that this is not good is hard :/

Joseph Silber wrote at 7/21/2015 9:03 am:

@bedroomation this is exactly why Laravel supports method injection on controllers. You can completely forgo the constructor, but still explicitly inject the methods dependencies.

Peter wrote at 7/21/2015 10:31 am:

Uhm, the main difference between a service locator pattern vs a dependency injection pattern is that you only depend on the locator/registry, and use that for resolving everything you need – if you want to avoid the locator, use a dependency injection pattern.

There is a lot of reasons not to use a service locator, and there is a few reasons why a service locator can be a good choice – either way, use one of the two, not both.

« »

Copyright © 2022 by Brandon Savage. All rights reserved.