Closed frodeborli closed 3 years ago
I'm not sure if I understand your question fully, but the high usage of this package (248M downloads as of now on Packagist) surely proves that this is useful.
As for not interacting with the container and using interfaces, yes, those are very good approaches. Your code should not be aware of the container, but just accept stuff through DI. The interop here is obtained at the "plumbing" level though, through frameworks (which is what the F in PHP-FIG stands for). This is also in the meta document of this PSR:
By standardizing the way entries are fetched from a container, frameworks and libraries using the Container PSR could work with any compatible container. That would allow end users to choose their own container based on their own preferences.
This seems more a discussion than an issue, I encourage you to discuss this further on our Discord server or in our mailing list.
The illogical thing I feel should be adressed, is that the PSR documentation contains an opinion that I can only understand as an argument declaring this PSR moot.
Unless libraries actually interact with the service container, the PSR only achieves defining some data structure belonging internally in some service injection component.
I implemented this PSR in my framework because I generally like interoperability. The HTTP PSRs for example, have increased interoperability in many ways, but 4 years since I implemented this PSR in our framework, I see no added benefit. It is popular because popular frameworks implement it.
I think PHP-FIG is short for "PHP Framework Interoperability Group".
How does this PSR improve interoperability?
@frodeborli you are considering the point of view of libraries.
This PSR is not for libraries. It is for frameworks.
This PSR standardizes the Service Locator pattern behind an interface. People like to call it an antipattern: it is if you use it in a project or a library. But frameworks have to use that pattern. Indeed, "dependency injection" doesn't happen magically.
So, to be clear, this PSR increases interoperability between frameworks and DI containers. Thanks to this PSR, frameworks can fetch services from any compatible DI container.
It is not meant to be used by end-user projects.
It is not meant to be used by libraries.
It is meant to be used by frameworks, which all together validated this PSR because they felt the need for this.
Hope that helps.
@mnapoli I see, and I understand. Thank you for the clarification. I was making some assumptions.
It would be interesting to see if this PSR has actualy caused any known framework to switch their bundled service container. The interface is so simple that our devs generally implement it themselves for various smallish projects. We started out with league/container in one project, but wound up replacing it with our own simpler implementation.
However, just hear my perspective as a library author: I feel that allowing library authors interact with container directly could improve interoperability, without neccesarily resorting to the service locator pattern.
Frameworks can easily provide a "proxy container" to libraries, which negotiates the dependencies, as an alternative/supplement to more classic DI implementations. Internally in the proxy container instance, the library could use all the dependency injection logic they would normally use.
It would provide a standardized way to retrieve all other PSR compliant implementations for third party libraries. If my library wants to create a ResponseInterface, it would call $proxyContainer->get(ResponseFactoryInterface::class) to get a PSR 17 response factory.
The DI implementation would be responsible for ensuring that it does not become a service locator.
This simply means that Container can also be used as a container which holds all the dependencies that I MIGHT want to use, instead of providing me with pre-constructed instances of services I might NOT use during a request cycle.
It is not neccesarily the service locator pattern (which we can agree is discouraged).
My problem is with the blanco "providing a container to libraries is a bad pattern" argument made in the documentation.
The consequence of this statement, is that library authors do something like this:
class DatabaseCache implements CacheInterface {
public function __construct(PDO $dbConn);
}
This pattern causes a database connection to be created, before we even know if the $cache->get()
method will ever be called during this request cycle.
Further down the line, some component expects the CacheInterface implementation to be injected, and quickly you end up in a situation where every request cycle end up connecting to the database, the memcached servers and instantiating perhaps a thousand instances of services that won't be used.
I think rephrazing the comment in the documentation would be good, to explain that the service locator pattern is generally discouraged, and that care should be taken to avoid it. One suggestion is that proxy containers is a good alternative.
In short If nikic/fast-route needs a CacheInterface, we provide it with a Container that contains only the CacheInterface we want it to use.
This is NOT the service locator pattern, but the psr/container documentation incorrectly say that it is.
OKKKK that is much more clear now. Thank you for explaining.
If I understand correctly, you're referring to:
1.3 Recommended usage
Users SHOULD NOT pass a container into an object so that the object can retrieve its own dependencies. This means the container is used as a Service Locator which is a pattern that is generally discouraged.
Please refer to section 4 of the META document for more details.
Your argument is that this section is actually hurtful.
This pattern causes a database connection to be created, before we even know if the $cache->get() method will ever be called during this request cycle.
Most database libraries, or actually any library with the same problem, usually start the connection lazily (not in the constructor). In practice, I don't think this is actually a problem.
Another option, sometimes used, is the ProxyManager library.
I think rephrazing the comment in the documentation would be good, to explain that the service locator pattern is generally discouraged, and that care should be taken to avoid it. One suggestion is that proxy containers is a good alternative.
TBH I don't think changing a PSR is an easy task. Maybe a small clarification could be done though. What would you suggest specifically?
If I understand correctly, you're referring to:
1.3 Recommended usage Users SHOULD NOT pass a container into an object so that the object can retrieve its own dependencies. This means the container is used as a Service Locator which is a pattern that is generally discouraged.
Exactly right.
Your argument is that this section is actually hurtful.
Yes.
Most database libraries, or actually any library with the same problem, usually start the connection lazily (not in the constructor). In practice, I don't think this is actually a problem.
The database connection is a very obvious example, which is why i picked it.
The cost might be less severe for a class that loads a bunch of configuration files, or parses a big json file in the constructor. But the cost is there. Having millions of requests every day, each wasting a few precious CPU cycles does sum up - even though it does not cause the database server to crash thanks to a proxy class.
Even lesser obvious is the chain reaction when an injected instance has dependencies injected and those dependencies have further more dependencies - all of which are constructed.
Using a single, central service container might be considered (by some developers) an antipattern, but at the same time a container does not have to be a singleton.
A dependency injection library can easily create new "mini-containers" which are passed into the library.
Another option, sometimes used, is the ProxyManager library.
Complexity is also an anti-pattern, imho.
I think rephrazing the comment in the documentation would be good, to explain that the service locator pattern is generally discouraged, and that care should be taken to avoid it. One suggestion is that proxy containers is a good alternative.
TBH I don't think changing a PSR is an easy task. Maybe a small clarification could be done though. What would you suggest specifically?
I can try to formulate something, but i think the entire "suggested usage" text seems opiniated and could be removed.
I can't find any good citations for the claim that service locator is a bad pattern at all. There are pros and cons, as with all patterns and nobody should assume a role as the arbiter of truth. Declaring one side as an anti pattern without citations, nuancing or explaination is not good in my opinion.
Wikipedia has one argument against service locators that relates to compiled languages and the benefit of compile time errors. In PHP we dont get compile time errors no matter how hard we try.
The other argument relates to testing and mocking, which is also based on a premise that we are unable to create a different Container instance for each library that has dependencies.
One can make good arguments against DI, because the increased plumbing required. In classic synchronous PHP, "plumbing" roughly translates into "code that runs on ever page view".
I think that providing a distinct container for each service is a very valid approach for injecting dependencies.
The get() method would on demand parse the dependency tree.
A Container does not have to be a Service Registry, even if it could be.
Do you want me to suggest an alternative text as a PR, or would you consider simply removing the opinionated section?
Do you want me to suggest an alternative text as a PR, or would you consider simply removing the opinionated section?
I'm not sure if any of these things is doable, and what the exact process is.
I believe entirely removing the section is a lost cause, that's too big of a change.
You may have more chances submitting a PR (it will be much easier for people to understand and project), and then bringing that topic up in the FIG mailing list.
Wikipedia has one argument against service locators that relates to compiled languages and the benefit of compile time errors. In PHP we dont get compile time errors no matter how hard we try.
I partially disagree on this, Symfony's container is compiled and they try very hard to make error surface at compile time to reach those advantages
You may have more chances submitting a PR (it will be much easier for people to understand and project), and then bringing that topic up in the FIG mailing list.
That's a good suggestion. As of now, this kind of change could be addressed as an Errata, following the bylaw: https://www.php-fig.org/bylaws/psr-workflow/
I believe entirely removing the section is a lost cause, that's too big of a change.
I am mainly talking about removing the sentence that asserts that the service locator pattern is considered bad: "This means the container is used as a Service Locator which is a pattern that is generally discouraged."
What authority on software architecture is able to generally discourage a widely used software design pattern? Either provide sources to academic research which provides such a conclusion, or remove it.
This sentence is right now simply a random opinion - which is why I think that it does not belong in any document that wants to establish standards.
Removing that sentence does not change contents of the PSR, and I would consider it a minor change.
However, i think interoperability would be improved if the psr didnt specifically discourage injecting containers at all unless there are other particular reasons? It is up to each framework to decide if it wants to encourage or discourage certain patterns. Those encouragements will hopefully (probably) be based on sound understanding of that particular frameworks architecture.
This last change would require removing the entire paragraph, and I agree it is a slightly larger change.
I think we understand your point.
Take it up with the FIG mailing list, as suggested. Nothing will happen from this discussion.
Wikipedia has one argument against service locators that relates to compiled languages and the benefit of compile time errors. In PHP we dont get compile time errors no matter how hard we try.
I partially disagree on this, Symfony's container is compiled and they try very hard to make error surface at compile time to reach those advantages
We may have different definitions about what compiling means.
I believe trying to turn PHP into something that it isnt, by throwing a lot of userland code at it and creating complex build processes might not be the optimal way forward.
Since this PSR discourage the service locator pattern, thereby making it so that libraries don't interact with the container, how does this PSR promote interoperability?
It seems to me that interoperability for libraries is achieved if library authors simply request interfaces in their constructor?