Class as parameter

[hpatoio]

Changelog

Class for service sonata.intl.locale_detector.request_stack is now defined via parameters. This allow easier overiting

Subject

Class for service RequestStackDetector defined via parameter

[ste93cry]

Although the PR is ok for me, I'm 👎 because of this issue. As SonataIntlBundle is a Symfony bundle, I would expect it to follow the same conventions

[hpatoio]

@ste93cry yes that's the new way to customize class in Symfony.

This PR is against a branch were all classes are customizable using a parameter so for consistency also sonata.intl.locale_detector.request_stack IMHO should be customizable using a parameter.

When the maintainer decide to move to the new standard it can then change all classes.

[soullivaneuh]

Class parameter are here for historical reason.

Now the Symfony best practices recommends to not use this. And we follow them.

AFAIK, if you really want to override the class, you can play with compiler passes for that.

Otherwise, if you think this class can be overrided for good reason, you should propose something with configuration, not class parameter.

Thanks for you contribution anyway. 😉

[greg0ire]

Now the Symfony best practices recommends to not use this. And we follow them.

@soullivaneuh No they do not, or at least, not for libraries. Here is what is written at the top of the BP document :

This guide aims to fix that by describing the best practices for developing web apps with the Symfony full-stack Framework.

[soullivaneuh]

@greg0ire https://symfony.com/doc/current/best_practices/business-logic.html#service-no-class-parameter

This practice was wrongly adopted from third-party bundles. When Symfony introduced its service container, some developers used this technique to easily allow overriding services. However, overriding a service by just changing its class name is a very rare use case because, frequently, the new service has different constructor arguments.

So indeed the top introduction talk about app for most of the cases, but on this one, third party bundles are concerned too.

So we can close it. 😄

[soullivaneuh]

Missread the sentence indeed, this is wrongly adopted from the bundle.

Anyway, I still think it's better to propose service id configuration than permit class override.

As for final keywords and private properties, this permits to have a better control of what can be and can not be overrided.

[greg0ire]

Closing the PR then, since I tend to agree that this practice is indeed kind of shady.