“Action Injection” As A Code Smell

Circumstance has conspired to put Action Injection discussions in front of me multiple times in the past few days. Having seen the approach several times before, I have come to think that if Action Injection is the answer, you might be asking the wrong question. I find it to be a code smell, one that indicates the system needs refactoring or reorganizing.

Action Injection …

As far as I can tell, the term “Action Injection” originates with Alex Meyer-Gleaves in a 2010 article on ASP.NET MVC development. He summarizes Action Injection in this way:

Your [controller class] constructor is provided the dependencies [by the DI container] that are shared by all actions in your controller, and each individual action [method] can request any additional dependencies that it needs.

To expand on that, let’s say you have a controller class with several action methods in it. You realize after a while that the different action methods have slightly different dependencies. For example, some of the methods need a logger, while others need a template system, or access to the router. But you don’t want to pollute the controller class constructor with these method-specific dependencies, since those dependencies will be used only if that particular action method gets invoked.

With Action Injection, when you pull the controller from your dependency injection container and call a particular action method, the DI container will automatically pass the right dependencies for the method call arguments. Voila: now you can define all the common dependencies as parameters on the controller constructor, and all the action-specific dependencies as parameters on the method definition.

You can see a PHP-specific description of the problem, with Action Injection as the solution, in this Symfony pull request:

https://github.com/symfony/symfony/pull/21771

You can also hear about it in this presentation from Beau Simensen, from around 32:21 to 34:31:

https://www.youtube.com/watch?v=JyrgwMagwEM&feature=youtu.be&t=1941

The Yii and Laravel containers appear to support this behavior as well, using the term “method injection.” (I think that’s a misnomer; the term appears overloaded at best, as sometimes it may mean “methods called by the DI container at object-creation time” instead of “resolving method arguments at call-time”.) Perhaps other DI containers support Action Injection as well.

… As A Code Smell

The explicit reason for using Action Injection is “to reduce dependencies or overhead” when constructing an object. You don’t want to have to pass in a dozen dependencies, when only three are used in every method, and the others are used only in specific methods.

But the fact that your controller has so many dependencies, used only in some cases and not in others, should be an indicator that the class is doing too much. Indeed, it’s doing so much that you cannot call its action methods directly; you have to use the dependency injection container not only to build the controller object but also to invoke its action methods.

Reorganizing To Avoid Action Injection

What approaches exist to help you avoid the Action Injection code smell? I assert that the better solution is to change how you organize your controller structures.

Instead of thinking in terms of “a controller class with action methods,” think in terms of “a controller namespace with action classes.” Actions are the targets for your routes anyway, not controller classes per se, so it makes sense to upgrade actions to “first-class” elements of the system. (Think of them as single-action controllers, if you like.)

Thus, instead of …

<?php
namespace App;

class BlogController {
    public function browse() { ... }
    public function read() { ... }
    public function edit() { ... }
    public function add() { ... }
    public function delete() { ... }
}

… reorganize to:

<?php
namespace App\BlogController;

class Browse { ... }
class Read { ... }
class Edit { ... }
class Add { ... }
class Delete { ... }

Then the DI container can use plain old constructor injection to create the Action object, with all of its particular dependencies. To invoke the Action, call a well-known method with the user input from the route or request. (I like __invoke() but others may prefer exec() or something similar.)

In fact, I realized only after watching the Simensen clip a second time that his example is a single-action controller in everything but name. His example code was this …

<?php
class Home {
    /**
     * @Route("/myaction", name="my_action")
     */
    public function myAction(
        Request $request,
        Router $router,
        Twig $twig
    ) {
        if (!$request->isMethod('GET')) {
            return new RedirectResponse(
                $router->generateUrl('my_action'),
                301
            );
        }

        return new Response(
            $twig->render('mytemplate.html.twig')
        );
    }
}

… but the controller action method parameters might just as well be Action class constructor parameters:

<?php
namespace Home;

class MyAction {

    public function __construct(
        Request $request,
        Router $router,
        Twig $twig
    ) {
        $this->request = $request;
        $this->router = $router;
        $this->twig = $twig;
    }

    /**
     * @Route("/myaction", name="my_action")
     */
    public function __invoke() {
        if (!$this->request->isMethod('GET')) {
            return new RedirectResponse(
                $this->router->generateUrl('my_action'),
                301
            );
        }

        return new Response(
            $this->twig->render('mytemplate.html.twig')
        );
    }
}

(UPDATE: That example code looks like it originates from Kevin Dunglas and his DunglasActionBundle — which is itself a single-action controller implementation for Symfony.)

Action Domain Responder

For more on this organizational structure, please read my Action Domain Responder offering. ADR is a refinement of MVC that is tuned specifically to server-side request/response over-the-network interactions.

But you need not go full ADR to avoid Action Injection. Just using single-action controllers will do the trick. Then you can have well-factored single-responsibility controller classes that do not require a DI container in order to call their action methods, and Action Injection becomes a thing of the past.

Are you stuck with a legacy PHP application? You should buy my book because it gives you a step-by-step guide to improving your codebase, all while keeping it running the whole time.
Share This!Share on Google+Share on FacebookTweet about this on TwitterShare on RedditShare on LinkedIn

13 thoughts on ““Action Injection” As A Code Smell

  1. In Yii we have method injection support in container i.e. ability to invoke method injecting objects got from container based on method signature. However, method injection isn’t applied to controller actions by default.

    We do have standalone action classes and I agree that these fit injecting dependencies much better than controller methods. Still, I think that controller’s grouping of *simple* inline actions is handy and easier to maintain than a number of separate action classes.

  2. A small correction not related to the general intention of the article, but a technical detail:

    Action Argument Injection is not a feature of the Symfony DI Container or the respective component. It is done by resolver classes at execution time, which is a feature of the HttpKernel that is in turn used and configured by the FrameworkBundle. The container happens to be the mechanism used to pull the resolved arguments from, but this is unrelated to the general way argument injection works in Symfony – the DIC does not inject arguments into action calls.

  3. Magento 2 takes the approach of separating actions into their own class files – likely for this exact reason.

    I’ve gotten very used to it and love it a lot. After reading Alexander’s post about Yii supporting that (which I didn’t know), I’m more excited than I was previously about using Yii in my personal projects

  4. Is there a reason as to why we use __invoke? Can’t we just dispatch to a handle method on the controller action class?

    For the record, I totally agree with this approach. We should route to a single class to handle that single responsibility. I just don’t like __invoke. Maybe I’ll change my mind on this eventually, or maybe there’s another way? I’d like to move to other languages and still route similarly.

    • > Is there a reason as to why we use __invoke?

      Not so much a “reason” as a “rationalization.” 😉 To wit: using __invoke() makes it easy to use “any callable” with the routing/dispatching system, so that the router/dispatcher does not care if you are targeting against a function, a closure, or a callable object. But the particular method name does not matter otherwise, so long as it’s well-known by whatever is calling it.

  5. I think it would great if we could simplify this even more and change it to use bare functions instead of class methods. There isn’t really a need to use actual classes, and my guess is that PHP autoloading makes it impossible to work with functions as we do with classes. That’s too bad…

    • > use bare functions instead of class methods

      I’ll see your “functions” and raise you “anonymous functions”. Slim works exactly that way. So, plenty of room for that approach if you don’t want to use classes!

  6. It will become particularly clear that this is including complexity whilst you consider what will occur when those Action instructions have code in not unusual. Do associated moves now want to proportion a trait? Or do you introduce inheritance in which related Action classes extends a common superclass? These answers are not objectionable in step with se but it does upload complexity.

    • This would likely indicate your controllers are doing too much, or they’re doing non-controller work. It’s time to factor whatever it is those controllers have in common into a service – this will simplify rather than complicate things, as those smaller classes with more well-defined responsibilities are easier to maintain. Eventually you learn to do this “ahead of time”, and you’ll start to see how much of a relief this really is. I’ve been using class-per-action exclusively for about two years now, and I’m definitely never going back 🙂

  7. Completely agree. Very real description about the motivation that goes in mind of a programmer right before he uses action injects.

    I have actually learned about this few years ago from Kevin’s bundle.

    I have ported the idea to Nette framework recently: https://github.com/Symplify/SymbioticController/blob/master/README.md#1-create-apppresenterscontactpresenterphp-presenter-with-__invoke-method

    And positive externality is IDE powered class routing (also in examples in Readme), so no more random string routes.

    As we talk about smell, there are sniffs for that: https://github.com/Symplify/Symplify/tree/master/packages/CodingStandard/src/Sniffs/Controller 🙂

    This is closest to became standard controller as ever was. Looking forward to the future. Great job Paul!

Leave a Reply

Your email address will not be published. Required fields are marked *