Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
do not add redundant method calls from interface injection#541
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
… already been set manually
lsmith77 commentedApr 13, 2011
@vicb: i am sure there could be some use cases, maybe we should just disable interface injection entirely if a manual call has been set? |
kriswallsmith commentedApr 13, 2011
I don't think we have any basis to determine whether a user wants a method to be called multiple times or only once. If this is an issue we should remove any interface injectors from the core. |
lsmith77 commentedApr 13, 2011
@kriswallsmith: well similar issues can happen with any 3rd party bundle that uses interface injection. so i think the approach of just disabling interface injection if a single manual method call is set, should work best here: aka setting a manual method calls means you want to manually manage setter injection. should make things clear in the docs too. |
kriswallsmith commentedApr 13, 2011
Symfony is built around the user being explicit about his intention and us not making any assumptions. This patch bakes in an assumption that the user only wants a method to be called once and creates an unnecessary limitation. |
fabpot commentedApr 13, 2011
I have reverted this patch for now as I think@kriswallsmith and@schmittjoh have good arguments. |
kriswallsmith commentedApr 13, 2011
I agree. Interfaces and parent definitions are “upstream” and should be prepended.
|
lsmith77 commentedApr 13, 2011
Well then you still have redundant method calls, which may or may not lead to simply adding overhead or actually causing issues if the second method call doesnt simply override the previous one. So I still think the best solution is to simply disable interface injection for any definition that defines an explicit method call. |
lsmith77 commentedApr 13, 2011
I have opened a pull that implements what I suggested:#544 |
lsmith77 commentedApr 13, 2011
closing this pull .. |
do not add method calls from interface injection if a method call has already been set manually
fixes issue caused by#535