-
Notifications
You must be signed in to change notification settings - Fork 12
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
Return a singleton instead of null when trying to create multiple factories #150
Comments
As a side note, I've implemented the last proposal at #151 because it seems to better comply with semantic versioning |
Hi fcoedno, First of all, thanks for suggestions on this particular behaviour. Secondly, we will apologise for how long it took to give you an answer. Again, thank you for your patience. |
@mmelograno do we have any expectations regarding the due date of it? Thanks in advance. |
Any updates? |
Hi @fcoedno, @diniz-victor and @boxofnotgoodery! First of all, sorry for the delay on this feature coverage. Second, we have a candidate version for this which you can test if you want by adding the following line This version is a breaking change due to we are allowing Multiple Factory Instantiation as well as decided to remove support for PHP 7.3 It would be awesome if you can confirm that this version accomplishes your expectations. Thanks, |
@mmelograno Returning a new instance for localhost is what we need to make our unit tests work. Now, we override the SDK factory method with almost exact code. So hopefully you'll release this branch at some point :) |
The current code returns null when a factory was already instantiated. I can see some major problems with this approach:
My suggestion would be: Instead of tracking the instantiation of the factories by using a flag (KEY_FACTORY_TRACKER) we would keep a reference to the instantiated object in the container, so that whenever an user of the SDK tries to create multiple factories, we simply return the last created factory
Another approach would be to create a new static method called singleton to make this behavior more explicit.
The text was updated successfully, but these errors were encountered: