Bundle controllers extending ContainerAware

[tgalopin]

Hi everyone!

In the Developer eXperience initiative, @weaverryan suggested that FOSUserBundle controllers should probably extend the Symfony Controller base class (FriendsOfSymfony/FOSUserBundle#1527). But the choice to extend ContainerAware instead of Controller was based on the documentation: http://symfony.com/doc/current/cookbook/bundles/best_practices.html#controllers.

Why is it a best practice to extend ContainerAware instead of Controller? In a bundle, we already are in the context of the framework, so it does not break any dependency and it allows developpers to use shortcuts provided by Controller in their bundles, which is great for beginners, so I don't get why.

Thoughts?

[xabbuh]

I don't see any subtle difference in extending the base controller or extending the ContainerAware class.It's just that you could theoretically use the controller in an app without having the FrameworkBundle in it. Though I doubt that you will have many applications that don't have this bundle.

What I would rather advocate here is to not inject the complete container into your controller classes, but to inject only the services you need instead. In my opinion, @matthiasnoback describes this really nicely in his blog.

[fabpot]

Extending the base controller is just fine.

[tgalopin]

@xabbuh The problem with injecting services is that we don't improve DX. The point of the first issue was to improve DX by using the base Controller class which provide useful shortcuts for beginners. It's better for new developpers if they can use the same shortcuts they usually use.

@fabpot Should we change the documentation to remove the paragraph then?

[matthiasnoback]

@xabbuh Thanks for mentioning my article. I think it might be good to educate people about this (and strengthen their "coupling radar"). However, when it comes to DX I'd recommend extending from the Controller class, because new users of Symfony2 already have so much to learn.

@tgalopin I definitely think that most of the "best practices" cookbook article doesn't make sense. Redistributing a bundle means preparing it for reuse in other projects that depend on the FrameworkBundle, so why not use its Controller class? Extending from ContainerAware is not a best practice in that case. So @weaverryan: I agree that FOSUserBundle should have controllers extending from Controller too.

By the way, if redistribution is your goal, then your bundle should contain almost nothing, except configuration files. All your "logic" should be in a separate library, which is reusable across frameworks.

[tgalopin]

"By the way, if redistribution is your goal, then your bundle should contain almost nothing, except configuration files. All your "logic" should be in a separate library, which is reusable across frameworks."

👍 with that

I create a PR to remove the paragraph.

[cordoval]

👍

[wouterj]

We already had an issue about it: #3672

@fabpot introduced that text, but looking at the comment he made here, I think he changed his mind on this.

[tgalopin]

(Misclick for my comment ...)

I didn't see this issue #3672. We will see how the PR go then.