AcclimateContainer / acclimate-container

Adapters for PHP framework containers to an interoperable interface
MIT License
220 stars 16 forks source link

Request for Feedback #3

Closed jeremeamia closed 10 years ago

jeremeamia commented 11 years ago

What do you think about Acclimate? Is it a good idea? Can you think of any good use cases for it? Any other thoughts, ideas, feedback, criticism, or praise you want to share? Please do.

taylorotwell commented 11 years ago

I like the interface. Simple, but still useful. Would like to see that style of interface in a PSR honestly.

igorw commented 11 years ago

I don't think a service locator interface is very useful, because it encourages over-relying on injecting the container. Which is usually something we want to get away from in the first place.

Ideally there would only be a single call to ->get(), in the front controller. Realistically we may need to inject it in 2-3 other places in order to get lazy loading. But after that, you should be injecting directly the things you need.

By having incompatibilities between containers, it actually highlights where you are injecting it (and probably shouldn't be). Just my $0.02.

taylorotwell commented 11 years ago

Yeah, I guess now that I think about it my main problem wouldn't be solved anyway. I've tried to use libraries that depend on different versions of Pimple, etc. or use some other container, and I end up with like 2-3 containers in my project, which feels gross. But, I guess a PSR interface alone doesn't really solve that.

jeremeamia commented 11 years ago

Use cases for Acclimate I've been thinking about:

Would love to here about any other ideas for use cases others have.

jeremeamia commented 11 years ago

@igorw I appreciate your feedback, and I would like to discuss your comments.

I don't think a service locator interface is very useful, because it encourages over-relying on injecting the container. Which is usually something we want to get away from in the first place.

I agree that injecting containers all over the place is bad, but I'm not sure that the existence of an interface like Acclimate's encourages this behavior any more than the existence of the various container implementations. I can understand if you don't think it's useful though, because I have had a hard time coming up with really compelling use cases myself. That's partially the reason I completed and published the project, so I can get feedback and see what ideas other people have.

Ideally there would only be a single call to ->get(), in the front controller. Realistically we may need to inject it in 2-3 other places in order to get lazy loading. But after that, you should be injecting directly the things you need. (emphasis added)

For the things that containers are injected into, should they rely on a particular container implementation? I think relying on the Acclimate interface (or some kind of common interface) can be of use, since the container implementation could be swapped out. Of course, that ability may or may not be valuable depending on what you are doing.

By having incompatibilities between containers, it actually highlights where you are injecting it (and probably shouldn't be).

I'm not sure I even understand what you meant here. :confused:

igorw commented 11 years ago

TLDR: I think this project is focussing on the wrong side of the container: How it is used. It should instead focus on how the data gets into the container, so that libraries can provide a generic config that can be imported into any container. This was already brought up on the fig list as well.

By incompatibilities, I just meant that we realized a few pain points with the different containers between silex and symfony. But those pain points actually make you think about why you are injecting the container in the first place, and allow you to come up with a better solution that does not require it. Instead of putting band-aid on it. Which is what I think this project is: A band-aid for overly relying on service location.

jeremeamia commented 11 years ago

TLDR: I think this project is focussing on the wrong side of the container: How it is used. It should instead focus on how the data gets into the container, so that libraries can provide a generic config that can be imported into any container. This was already brought up on the fig list as well.

I think how it is used is also important, but I agree that there could be work done to focus on the configuration/setting aspect as well. It might be something I end up looking into later. Unfortunately, it's a much more difficult problem and involves catering (or not catering) to a lot of different opinions.

Well, I guess I should change the tagline of this repo to "A band-aid for overly relying on service location". LOL. I knew I could count on you for some good criticism. :wink:

moufmouf commented 11 years ago

Hi Jeremy!

Want some feedback? Easy! Acclimate is huge! I think you are going in exactly the right direction.

Acclimate is the tool I was needing to start writing "DIC" agnostic applications. Today, all MVC frameworks are tightly coupled with their DIC framework. Have a look at Silex! The main class directly extends Pimple! We need to loosen that coupling, so that innovative DI containers can come up to light and be used.

Now, I think you need a use case to prove Acclimate can be useful. Luckily, I might help you with that ;)

Check this: https://github.com/thecodingmachine/interop.symfony.di

This is an extension to the Symfony 2 container that let's you add additional containers (as a fallback container) should Symfony DIC not find the instance it is looking for. Now, any container added must respect the ContainerInterface interface, but thanks to Acclimate, I can now inject any DI container, right into my Symfony 2 application!

(I'm using it to add Mouf: http://mouf-php.com , and this gives me a graphically editable DIC in Symfony. Nice!)

Note: this code has been written yesterday and today, so it is still quite fresh. Fill free to give it a try anyway!

moufmouf commented 11 years ago

And here is another project that I wrote a few weeks back and that I just adapted to fit Acclimate:

https://github.com/thecodingmachine/interop.silex.di

It let's you create a Silex application that uses not only Pimple, but any other DIC, thanks to Acclimate!

Now, you can register any adpater from Acclimate directly into Silex using:

$app->registerFallbackContainer($myAdapterFromAcclimate);

I feel the Silex sample is great. I mean, Silex is a very pleasant micro-framework, and when your Silex app starts to grow, the first thing that is getting messy is the Pimple configuration. Being able to push that configuration in a more solid container is a great use case of Acclimate.

harikt commented 11 years ago

There is a similar question in stackoverflow http://stackoverflow.com/questions/7628936/whats-the-best-way-to-integrate-an-event-dispatcher-in-a-php-library/

Probably @odino will be having much better feedback for this I assume.

mnapoli commented 11 years ago

Very interesting project, of course I'm very much in favor of a PSR for the interface and I think it's a good proof.

@igorw the target user for Acclimate is (IMO) not end-users (developers) but frameworks. If a framework is built against a ContainerInterface, then users can give it another container of their choice. A framework will use a container as a service locator, and that's OK. A user however shouldn't. (that why also I think the interface should be named ServiceLocatorInterface).

Standardizing "how you put data in the container" is nearly impossible today as all container are different on this point, and also it's the point. Remove these differences and you end up with 1 implementation.

odino commented 11 years ago

+1 to @mnapoli

jeremeamia commented 10 years ago

Hey @moufmouf, @AmyStephen, @mnapoli, and @dongilbert. I just made a bunch of changes to Acclimate. This includes renaming a lot of classes/namespaces and moving the ContainerInterface to its own package. I'm wondering how you feel about the direction I'm taking, and if you have any ideas/feedback. I also feel like there might be some duplication of effort between Acclimate and container-interop, and I'm wondering what you think we should do about it.

AmyStephen commented 10 years ago

Agree @mnapoli

@jeremeamia +1 on packaging those Interfaces separately. Would love to see that become the norm in 2014.

In terms of the collaboration, we really do need to keep pushing on that. IMO, that's why we need a common namespace -- or a common repository -- or some way of joining these efforts together. Please do find a way to join your efforts. Splitting out the Interface into it's own package, like you did, should help.

moufmouf commented 10 years ago

Hi @jeremeamia!

I wish we had realized earlier that you were separating the interfaces from your code. We could have talked about it earlier. Of course, I think you are heading in the right direction (since we are definitely heading in the same direction)

We tried with @mnapoli to put the simplest possible interface in container-interop. And we followed the same path you followed, and we reached the same place :)

One of the important aspects of container-interop is that is carries no "brand". It is not branded Acclimate, not branded PHP-DI, Mouf, or whatever. This is something we believe is important and we want to reach this "unbranding" before a PSR gets voted (some other people on the FIG mailing list have strong positions believing that a package should not have a generic vendor name, we don't...)

We are still in the process of discussing the name of the interface, but discussions are getting traction so we might reach a consensus soon.

Would you consider switching Acclimate to use "container-interop" when discussions have ended and interface is stable? Of course, I'm sure @mnapoli would have no objection to granting you admin access to "container-interop". The whole point of this is joining our efforts together, showing there is momemtum, and if we are lucky having a PSR :)

mnapoli commented 10 years ago

Hi @jeremeamia!

Yes it's too bad we had no idea about this, this is really cool!

Indeed, there's a bit of duplication of effort for this given we are providing the same interface. However the goal of container-interop is to also try other things that could help for container interoperability. For example, we'd like to add soon a ChainableContainerInterface, and see if there are other useful interfaces to add (that may not be directly useful for Acclimate for example).

However I don't think it's relevant for container-interop to provide anything like Acclimate, since its already done and works pretty well. But there is Acclimate to make containers work together, there are also projects like Symfony & Silex adapters (made by @moufmouf), so there could be a bunch of tools (I'm thinking DI configuration adapters/translators for example) that could be made around the global idea of container interoperability but that may not be directly related to Acclimate. So I still think that the container-interop project can be useful: as a sandbox, but also as a rallying point/community of container developers.

And I agree with @moufmouf that having a non-branded organization and project at the core is also a good thing (I think however we should link to tools like Acclimate on container-interop). And btw we will grant admin rights to anyone involved of course, most of the work is done via pull requests anyway so that everybody can have a say.

Sorry for all this probably-unreadable bunch of text, had an intense day ;)

jeremeamia commented 10 years ago

Acclimate v1.0.0 is now available! Acclimate also now uses the container-interop ContainerInterface, instead of acclimate/api. Let me know if you are someone else you see ends up using Acclimate for something interesting. :smile:

moufmouf commented 10 years ago

Yeah! Congratulations for that v1.0!