stackphp / session

Session stack middleware.
MIT License
27 stars 6 forks source link

Building the container inside the middleware #3

Open alsar opened 11 years ago

alsar commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

@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 commented 11 years ago

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 commented 11 years ago

@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 commented 11 years ago

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.