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

Add a 'signature' method to ServiceProviderInterface #93

Merged

Conversation

greg-1-anderson
Copy link
Contributor

The existing ServiceProviderAggregate invariantly uses get_class to keep track of each provider to quickly and efficiently check to see if it has already been registered at some point in the past. This implementation prevents the use of the same Provider class to register services for multiple service providers.

For example, in instances where an application has a number of providers, all of which register services in a similar way, then a single simple service provider, such as SimpleServiceProvider could potentially be used to handle all of them. (Example usage).

This technique works fine for the first module, but the second use of the same provider is not registered due to the get_class check in ServiceProviderAggregate.

This PR adds a signature method to the ServiceProviderInterface, so that providers such as the SimpleServiceProvider may provide some different signature than its class name. The AbstractServiceProvider provides a default implementation of signature that simply returns get_class, so existing providers that extend AbstractServiceProvider will continue to function in the same way they always have.

The one downside of this implementation is that any provider that implements ServiceProviderInterface without extending AbstractServiceProvider will break, and will need to add a signature method in order to work again. This adjustment is trivial, but does break backwards compatibility.

If the concept of this PR looks acceptable, but strict backwards compatibility is desired, then I could add another ServiceProviderSignatureInterface, and test for it in ServiceProviderAggregate with a fallback to get_class for providers that do not implement the new interface. Such an implementation would be slightly more awkward than what is presented here, but would maintain strict backwards compatibility.

…e Provider class may be used to register services for multiple service providers.
@greg-1-anderson
Copy link
Contributor Author

Note that for the purpose of comparison, I made a separate PR, #94, that preserves backwards compatibility. The extra interface seems to have minimal impact overall, so perhaps that implementation will be preferable.

@greg-1-anderson
Copy link
Contributor Author

Another alternative would be to not use a Service Provider in this instance, and simply register the services to the container directly. Perhaps folks only use Service Providers when the register function is lengthy. I kind of like the service provider model for organizational purposes, though, and this technique would also be easily adjusted if / when things got more complicated (e.g. by replacing a Simple Service Provider with a specific provider implementation).

Your higher-level perspective on these issues would be appreciated, though.

@philipobenito
Copy link
Member

#94 (comment) hopefully this is agreeable, I would like to keep this open though and revisit it when 3.x starts.

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

Successfully merging this pull request may close these issues.

2 participants