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

Bug: Baseservice - getSharedInstance #2414

Closed
tada5hi opened this issue Nov 18, 2019 · 4 comments
Closed

Bug: Baseservice - getSharedInstance #2414

tada5hi opened this issue Nov 18, 2019 · 4 comments
Labels
bug Verified issues on the current code behavior or pull requests that will fix them

Comments

@tada5hi
Copy link
Contributor

tada5hi commented Nov 18, 2019

Direction
BUG

Describe the bug
Shouldn't the "getSharedInstance" of the system/Config/BaseService.php return an unique instance for name and params combination?
Currently it just reference to the same object instance whether the method params are equal or not.

CodeIgniter 4 version
Head

Affected module(s)
Which package or class is the bug in, if known.

Expected behavior, and steps to reproduce if appropriate
A clear and concise description of what you expected to happen,
and how you got there.
Feel free to include a text/log extract, but use a pastebin facility for any
screenshots you deem necessary.

Context

  • OS: [e.g. Windows 99]
  • Web server [e.g. Apache 1.2.3]
  • PHP version [e.g. 6.5.4]
@tada5hi tada5hi added the bug Verified issues on the current code behavior or pull requests that will fix them label Nov 18, 2019
@dafriend
Copy link
Contributor

dafriend commented Nov 19, 2019

Shouldn't the "getSharedInstance" of the system/Config/BaseService.php return an unique instance for name and params combination?

No, because if it's unique then it's not a "shared" instance. The whole purpose is to return an existing instance of the named class if it exists or instantiate one if it does not.

If you want a unique instance of something then call the Services method with getShared set to false e.g.

$renderer2 = \Config\Services::renderer($viewPath, $config, false);

@lonnieezell
Copy link
Member

@dafriend Nailed it.

@tada5hi
Copy link
Contributor Author

tada5hi commented Nov 19, 2019

But consider the following example, lets say i want to realize an authorization and i would do it the way @lonnieezell do it in myth auth. Than his function authentication($lib = 'local', ....)would always return the first initialized Authenticator, which would be the local authenticator.

But lets say you want to implement an Facebook & Twitter Authenticator then you would have different controller, which should return the facebook instance because ( in case of facebook endpoint) you want a shared instance in the controller and in the view or antoher library where you print the authorization url which is generated by your authenticator for example.

So i think it would be great maybe through another function name getSharedInstanceWithParams(...) instead of getSharedInstance(). Cause of the modularity i can implement this again just for me, but maybe also useful for others too.

@lonnieezell
Copy link
Member

In that situation you would have a factory method on the Authentication library itself. So the service would always return an authentication library that contains a factory method to make it easy to specify which driver it's using.

While doing it in the framework could be useful, it could also be problematic and have undesirable side-effects in other situations. So it's not something we'd want to consider at the moment. Especially as we're trying to get a final release pulled together.

This might be considered as a feature request for future versions if you'd like to bring it up on the forums and get some discussion going there. But, currently, it's not something we consider a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

No branches or pull requests

3 participants