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

Shared constructor #107

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Nemoden
Copy link

@Nemoden Nemoden commented Jun 7, 2016

"Share" is a nice option of Dice that guarantees not to create an object, but reuse already created one. But I encounter a lot of situations when I want to get the same object only if constructor parameters are the same. Consider common task like:

$dataFinder = $dice->create("DataFinder", [$model, $id]);
$dataFinder->getData();

Suppose DataFinder can cache result for searching a record id $id for a model $model.
If we use "shared" rule of Dice, we will always get same instance of DataFinder despite that we might want to get an instance of DataFinder for the particular $model and $id.

This pull request adds sharedConstructor rule so you can tell Dice not to create an object if an object of this type was already created with this particular constructor params.

In my practice this patterns occurs so many times... I usually do a caching proxy which is okay but it would be nice if we had such a feature in Dice right out of the box.

@Nemoden
Copy link
Author

Nemoden commented Jun 7, 2016

P.S. I am sorry for commit names, I've pushed first commit and than added extra tests and messed with the commit message trying to amend previos commit, but made a new one with the same msg :(

@Nemoden
Copy link
Author

Nemoden commented Jun 7, 2016

P.P.S. I am also not sure about the name of the feature "sharedConstructor" seems a little misguiding.

@TRPB
Copy link
Member

TRPB commented Jun 7, 2016

There so few cases where you should be calling create directly (ideally in just one place) that the usefulness of this should be incredibly limited. If you find yourself calling $dice->create in multiple places I'd suggest it's more likely a SRP issue.

Is there a reason that named parameters cannot be used here?

$dataFinderA = [
    'instanceOf' = 'DataFinder',
    'constructParams' => [$model, 'a']
];

$dataFinderB = [
    'instanceOf' = 'DataFinder',
    'constructParams' => [$model, 'b']
];

$dice->addRule('DataFinderA', $dataFinderA);
$dice->addRule('DataFinderB', $dataFinderB);

$dfA = $dice->create('DataFinderA');



It's  also incredibly easy to automate this. Wherever you currently have `$dice->create` replace it with:

```php
if ($dice->getRule('DataFinder' . $id) != $dice->getRule('*') {
    $dice->addRule('DataFinder' . $id, [
        'instanceOf' => 'DataFinder',
        'constructParams' => [$model, $id]
       'shared' => true
    ]);
}

$dataFinder = $dice->create('DataFinder' . $id);

I think if this is really needed, a better and much simpler solution is wildcard names. You'd assign the rule as normal:

$dataFinder = [
    'shared' => true
]

$dice->addRule('DataFinder, $dataFinder);

But when creating the object, provide a unique ID, perhaps something like:

$dice->create('DataFinder:' . $id, [$model, $id]);

Which, when $id is 123 would use the instance name DataFinder:123 but create an instance of DataFinder.

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