KnpLabs / KnpMenuBundle

Object Oriented menus for your Symfony project.
http://knplabs.com
MIT License
1.4k stars 204 forks source link

Implementation imposes anti-DI anti-pattern on implementors with parameterless constructor #443

Closed Bilge closed 3 years ago

Bilge commented 3 years ago

BuilderAliasProvider requires implementations to accept a parameterless constructor.

https://github.com/KnpLabs/KnpMenuBundle/blob/e1b1e74c4ce41dbe4bd9d4e78a918065cfc06c49/src/Provider/BuilderAliasProvider.php#L122

This implies dependency injection cannot be used, instead insisting dependencies are made public and pulled out of the container.

https://github.com/KnpLabs/KnpMenuBundle/blob/e1b1e74c4ce41dbe4bd9d4e78a918065cfc06c49/src/Provider/BuilderAliasProvider.php#L123-L125

This has long been considered an anti-pattern and dependency injection should be used instead but this library prevents that.

stof commented 3 years ago

The solution is simple: don't use the BuilderAliasProvider. Instead, register your builder class as services tagged as knp_menu.menu_builder. this gives you the full power of DI: https://symfony.com/bundles/KnpMenuBundle/current/menu_builder_service.html

garak commented 3 years ago

This is not imposed. As you can read on documentation, this is provided only to offer a "simple" (yet less formally correct) way to build a menu. The same documentation explains how to properly build a menu with dependencies.

stof commented 3 years ago

note that the note in the doc saying that the service must be public is not accurate anymore. When using Symfony 3.3+, builder services can be private

Bilge commented 3 years ago

Thanks @stof