8000 [FrameworkBundle] added ContainerAwareInterface to services.xml by schmittjoh · Pull Request #535 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[FrameworkBundle] added ContainerAwareInterface to services.xml #535

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 13, 2011
Merged

[FrameworkBundle] added ContainerAwareInterface to services.xml #535

merged 1 commit into from
Apr 13, 2011

Conversation

schmittjoh
Copy link
Contributor

No description provided.

@fabpot fabpot merged commit 61dba2d into symfony:master Apr 13, 2011
@lsmith77
Copy link
Contributor

Sorry, I didnt realize this before.

liip_hello.phpcr.controller:
    class: Liip\HelloBundle\Controller\PHPCRController
    calls:
        - ['setContainer', [ @liip_hello.container ] ]

Now leads to

protected function getLiipHello_Phpcr_ControllerService()
{
    $this->services['liip_hello.phpcr.controller'] = $instance = new \Liip\HelloBundle\Controller\PHPCRController();

    $instance->setContainer($this->get('liip_hello.container'));
    $instance->setContainer($this);

    return $instance;
}

I guess we should ensure that interface setters are either called first or better yet we need to remove redundant calls?

@lsmith77
Copy link
Contributor

The following patch fixes the issue, basically it will not add a method call via interface injection if the method has already been explicitly set:
http://pastebin.com/80GNjtK7

I am not sure if there is a sane use case where one would want to do multiple method calls via interface injection.

@fabpot
Copy link
Member
fabpot commented Apr 13, 2011

Your patch looks good to me. Can you send a PR?

@vicb
Copy link
Contributor
vicb commented Apr 13, 2011

@lsmith77 could we imagine something like a FileLocatorInterface resulting in multiple calls to addIncludePath ?
(not a very good example though)

@lsmith77
Copy link
Contributor

done here #541

@lsmith77
Copy link
Contributor

Well in this particular case the point is to "fix" the hardcoded dependency set by the interface injection.

@lsmith77
Copy link
Contributor

Dependency injection is all about being able to inject what you want. Implementing an interface is a way for the class to ask for some dependencies, but final control over this should always be with the end user using the class and not with the class itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
< 51D5 span class="css-truncate"> No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@schmittjoh @lsmith77 @fabpot @vicb
0