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

What to do with singletons? Feature or README? #74

Closed
frederikbosch opened this issue Nov 20, 2014 · 12 comments
Closed

What to do with singletons? Feature or README? #74

frederikbosch opened this issue Nov 20, 2014 · 12 comments

Comments

@frederikbosch
Copy link
Contributor

What is the best way to register a singleton through the Aura DI container? Let's say PDO. There should be only one instance of PDO through the whole request (in our app), but it might be required to inject it in multiple adapters.

Now I am using types for this purpose.

$di->set('pdo', function () { /* configure PDO */ });
$di->types[PDO::class] = $di->lazyGet('pdo');

It is not clear how to do this with the current README, or am I missing something?

Hope you can help. Thanks for the hard work: much appreciated.

@harikt
Copy link
Member

harikt commented Nov 21, 2014

@frederikbosch yup $di->set('<name>') .

$di->params['PDO'] = array(
    'dsn' => getenv('DB_DSN'),
    'username' => getenv('DB_USERNAME'),
    'passwd' => getenv('DB_PASSWORD'),
);
$di->set('connection', $di->lazyNew('Aura\Sql\ExtendedPdo'));

Now you can use $di->lazyGet('connection') where ever you need to inject the pdo instance. Please note the passwd for the variable. If you use reflection to get the variable name used internally of PDO you will get passwd iirc .

Let me know if that helps .

Thank you.

@frederikbosch
Copy link
Contributor Author

@harikt Thank you for your response. But I think that I was not completely clear. I want to register a singleton, so when I instance a new class through auto-resolution, it uses the singleton automatically.

So if we combine both our examples.

// register the services somewhere
$di->params['PDO'] = array(
    'dsn' => getenv('DB_DSN'),
    'username' => getenv('DB_USERNAME'),
    'passwd' => getenv('DB_PASSWORD'),
);
$di->set('connection', $di->lazyNew('Aura\Sql\ExtendedPdo'));

// is this the valid method for registering a singleton?
$di->types[PDO::class] = $di->lazyGet('connection');

// a class that has a PDO dependency
class SomeClass {
public function __construct(PDO $pdo) {}
}

// somewhere else, using auto-resolution here
$instance = $di->newInstance(SomeClass::class);

Now $instance should have the (PDO) singleton that I have registrered. This actually works with $di->types, but I believe I am abusing ->types there.

Perhaps PDO is not the best example. Other example: I have a PluginManager which is injected in some controllers. There should be only one PluginManager in my app. When I put the PluginManager dependency in the constructor of the controller, I want to have that one PluginManager automatically (auto-resolution) and not a new instance. How to achieve this?

@harikt
Copy link
Member

harikt commented Nov 21, 2014

@frederikbosch Thanks for clarifying.

Then your type is nice and there is no other way I am aware of.

@harikt
Copy link
Member

harikt commented Nov 21, 2014

Especially for you are trying to make use of auto resolution.

@frederikbosch
Copy link
Contributor Author

Thank you for the explanation. Why not creating a $di->singleton property. My general feeling is that $di->types is meant for defining which adapter to use for an interface and should not be abused for singletons. What do you think?

@harikt
Copy link
Member

harikt commented Nov 21, 2014

for the current time, I don't know :) .

@frederikbosch
Copy link
Contributor Author

If the feature is agreed on, I am prepared to work on this and issue the pull request.

@harikt
Copy link
Member

harikt commented Nov 21, 2014

@frederikbosch I am not sure for a few reasons.

  1. Auto resolutions seems hard when dealing with single tons or shared instances are needed.
  2. That have brought to turn on / off the feature.
  3. In next major version it may not be there. (Update : I guess)
  4. I could not comment for @pmjones for what he thinks about this in the current (2.x) version.
  5. I am ok with this.

You can always send a PR and wait for Paul to hear / merge it.

@frederikbosch
Copy link
Contributor Author

@harikt I will wait to see what @pmjones has to say. When there is no response, I might spend time on it. If he is also ok, I will for sure send a PR.

Regarding the API of the feature, this could be $di->singleton, but also $di->shared or something similar.

@harikt
Copy link
Member

harikt commented Nov 21, 2014

👍

@pmjones
Copy link
Member

pmjones commented Mar 15, 2015

After reading all the comments here, I think this is a documentation fix. "Singletons" are already available as "shared services" via set()/get(), so the main problem appears to be auto-resolving them. I can add something to the README easily enough.

@frederikbosch
Copy link
Contributor Author

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants