auraphp / Aura.Di

Dependency Injection System
MIT License
349 stars 63 forks source link

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

Closed frederikbosch closed 9 years ago

frederikbosch commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

@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 commented 9 years ago

@frederikbosch Thanks for clarifying.

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

harikt commented 9 years ago

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

frederikbosch commented 9 years ago

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 commented 9 years ago

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

frederikbosch commented 9 years ago

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

harikt commented 9 years ago

@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 commented 9 years ago

@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 commented 9 years ago

:+1:

pmjones commented 9 years ago

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 commented 9 years ago

:+1: