Skip to content

Improved the advantages/drawbacks of "controllers as services" #7495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 21, 2017
Merged

Improved the advantages/drawbacks of "controllers as services" #7495

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions controller/service.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,19 @@ These are the main **advantages** of defining controllers as services:
service container configuration. This is useful when developing reusable bundles;
* Your controllers are more "sandboxed". By looking at the constructor arguments,
it's easy to see what types of things this controller may or may not do;
* If you're not passing some required dependencies or if you are injecting some
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is worth clarifying the semantic above, because technically this is not the constructor of a controller but of a class of controllers, in this way a Dunglas action is more a controller than the base controller class.

non-existent services, you'll get errors during the container compilation
instead of during runtime execution;
* Since dependencies must be injected manually, it's more obvious when your
controller is becoming too big (i.e. if you have many constructor arguments).

These are the main **drawbacks** of defining controllers as services:

* It takes more work to create the controllers because they don't have
automatic access to the services or to the base controller shortcuts;
* It takes more work to create the controllers and they become more verbose
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really agree, controller's method should be as thin as possible and just calling services to do things to end by returning a response (so I've been told :). If services are there no need for shortcuts anymore. In the end you're just calling explicitly some methods, some would say "without magic" :D.

Again the semantic does not look appropriate because from a DI point of view defining an object makes it a service, but orchestrate an action requires to call every needed services, hence the need to access them.

The way we always used in Symfony is by making the controller class container aware, but defining explicitly the needed dependencies is not that different since the controller class is just virtual, what's truly needed is a locator and to group actions by scopes.

The new locator feature is awesome for this, and Symfony is great for letting every ways of coding possible, because they are all powerful in their way, one just need to do and learn what he can and want to do with it.

Personally I don't prefer one way or another, as long that it makes sense to do what you got to do.

By letting everyone playing with it some ways may die naturally on the long run.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry but I don't understand your comment. Let me show you why I wrote that phrase.

For such a common thing as rendering a template, if you use the base controller, you do this:

return $this->render('homepage.html.twig');

Just 1 short and beautiful PHP line.

However, if you don't use that base controller, the equivalent code is this one:

        if (!$this->container->has('twig')) {
            throw new \LogicException('...');
        }
        
        $response = new Response();
        $response->setContent($this->container->get('twig')->render('homepage.html.twig'));

        return $response;

So, I think the previous phrase is correct: "It takes more work" ... and "they become more verbose".

Copy link
Contributor

@HeahDude HeahDude Feb 28, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if the Twig environment is a hard (or safe) dependency, it could just be:

return $this->twig->renderResponse('homepage.html.twig');

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the working example would be:

private $twig;

public function __construct(\Twig_Environment $twig)
{
    $this->twig = $twig;
}

return $this->twig->renderResponse('homepage.html.twig');

So, again, "it takes more work" and "they become more verbose" 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The class is but not the controllers in it :p

because they don't have automatic access to the services and the base
controller shortcuts;
* The constructor of the controllers can rapidly become too complex because you
must inject every single dependency needed by them;
* The code of the controllers is more verbose because you can't use the shortcuts
of the base controller and you must replace them with some lines of code.
must inject every single dependency needed by them.

The recommendation from the :doc:`best practices </best_practices/controllers>`
is also valid for controllers defined as services: avoid putting your business
Expand Down