This repository has been archived by the owner on Feb 6, 2020. It is now read-only.
Removed $checkAbstractFactories flag from has() #49
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The flag breaks the LSP, and requires that a user know how a given service might be configured inside the container. Removing it provides easier interoperability, and removes unexpected false negative lookups.
- As this patch makes it obsolete.
|
+1 for this change. This makes the signature coherent with container-interop contract, and as said in the other, the performance penalty will be negligeable anyway. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In updating zend-mvc to v3 of zend-servicemanager, I ran into a subtle issue:
has()does not look in abstract factories by default, and requires that you pass an additional flag,$checkAbstractFactories, with a boolean true value, if you want to test if an abstract factory can resolve the given$name.This breaks the Liskov Substitution Principal, as it means that the behavior of
has()depends on how the ServiceManager is configured. Users then must be aware that an abstract factory might supply a dependency, and cannot rely onhas()per the container-interop contract to work for the specific use case of zend-servicemanager.As such, this patch removes the flag, and checks abstract factories whenever a service or factory matching the provided
$nameis not found.