[2.1] Method Injection #881
Conversation
| // enable method injection | ||
| $passes = $passConfig->getRemovingPasses(); | ||
| array_unshift($passes, new GenerateLookupMethodClassesPass($this->getCacheDir().'/lookup_method_classes')); | ||
| $passConfig->setRemovingPasses($passes); |
There was a problem hiding this comment.
Why add the pass here instead of in the container?
|
I've read the blog article from 2004 and still disagree with this feature (as I've articulated in #835). The idea of native support for lazy injection is intriguing, but that would be better implemented by wrapping the dependency in a proxy object. I don't see any case when abstract lookup methods would be a best practice, unless you're designing a class for this particular feature. I would rather we design the DIC to work with classes designed without any knowledge of the container than provide features that are only useful if you design a class to be used with the container, thus binding your class to the DIC. |
|
Well I think we can all agree that injecting the entire container is an ugly practice, especially for Controllers since they are obviously quite central to a given request. Its ugly because it is a super tight coupling with the DIC and specific services. However to keep things simple we decided to default to using this approach. Now the question is if we want to add even more "questionable" tools to make this practice a bit less ugly or if we instead try to encourage users to move towards Controllers as services if they care about reducing the coupling (I agree with @schmittjoh that this approach actually reduces the coupling rather than increases the coupling if you are already injecting the DIC). We could also at least define that adding a getter for a service is a best practice to follow when injecting the container. Which is essentially what this PR tries to encourage by reducing the amount of code needed. BTW: I think phpStorm 2.1 (and maybe some other IDE's) also support auto completion for Anyway, to me this eases the pain we have with a problem we shouldn't have to begin with so I am still not very enthusiastic about this. The problem is solved more elegantly by the ContainerWrapperBundle albeit with potentially more overhead in performance of course. |
|
Let's schedule the discussion for 2.1. |
Implements #835