Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Building the container inside the middleware #3

Open
alsar opened this issue Jul 18, 2013 · 5 comments
Open

Building the container inside the middleware #3

alsar opened this issue Jul 18, 2013 · 5 comments

Comments

@alsar
Copy link

alsar commented Jul 18, 2013

I was a bit wondering why the Pimple container is build inside the middleware? Wouldn't it be better, if the required objects would be passed from the outside?

I just want to know what's the reason for this.

@simensen
Copy link
Member

The fact that Pimple is used at all is an implementation detail. Users of the Session middleware should be unaware that Pimple is used at all but I'll admit that it is possible that dependency may leak. :)

A middleware should be standalone and have its dependencies passed to it. Using some sort of shared container (Pimple or otherwise) amongst middlewares would start to put extra dependencies on each middleware and could eventually lead to far coupling than is desired.

I'm sure @igorw has other reasons and his may contradict mine ( :) ) but this is how I understand it. I'm going to be taking this same approach in at least one middleware I'm working on and these are the reasons I've decided to go down that road.

@davedevelopment
Copy link

@simensen I don't think he was caring for shared containers, I think he was alluding to injecting instances of things into the middleware, like a different storage handler.

@alsar you can still pass in some dependencies, but it's a bit wishy washy, regardless of whether it's advertised in the readme, anything in the options array will overwrite anything in the internal pimple container.

I've mixed feelings on this. Without this 'backdoor', you lose inversion of control, but you are also exposing the internals of something you didn't want to expose.

@igorw
Copy link
Contributor

igorw commented Jul 18, 2013

I agree pretty much with what @simensen said, and that is the motivation behind the current design.

That said, as soon as you want to override services, those details leak out, and I'm not sure if there is a good way to fix that.

Users should not have to wire up all the dependencies since it easily becomes a bootstrapping problem. To elaborate on that, these middlewares are meant to be used (and configured) in the front controller. That means that they need to be usable. It also means that the configuration will most likely happen before the application is bootstrapped, which means you cannot easily depend on your framework's containers or config files.

Yes, we should try to find a way to avoid pimple details from leaking out. Thanks for brining this up!

@alsar
Copy link
Author

alsar commented Jul 18, 2013

@davedevelopment Yes i had inversion of control in mind. Because now the middleware has a hard dependency on the container. Pulling dependencies from a container is always like some sort of black magic.

@alsar
Copy link
Author

alsar commented Jul 18, 2013

It's now starting to make sense to me, why it's designed the way it is.

@igorw I agree with you. Middlewares should not (and cannot) depend on the container of the underlying application. They must be self sufficient. But regardless, there must be a clean solution that the dependencies are injected from the outside.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants