Prototype scope on "factory" definitions #164

Closed
avant1 opened this Issue Jun 3, 2014 · 3 comments

Comments

Projects
None yet
2 participants
@avant1
Contributor

avant1 commented Jun 3, 2014

Hello.
In my app I use constructor injection, and I define it using \DI\ContainerBuilder::addDefinitions() method.
One of injected objects is injected as a factory, and this object is randomly configured in factory.
But when I get this object in constructors, the object is always same. Usually it is ok, but I would like to disable this behavior for single definition.

There is method \DI\Container::make(), but I don't know how to use it without passing container object to constructors where my randomly object needed.

Maybe there is any way to disable caching for single definition, or use make() instead of get() when constructor dependencies are created? Or should I refactor my application anyway?

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Jun 3, 2014

Member

Hi! I think I see what you problem is, let me know if I'm wrong.

You have something like this:

return [
    'Foo' => DI\factory(function () {
        return new Foo();
    }),
];
$foo1 = $container->get('Foo');
$foo2 = $container->get('Foo');

Here $foo1 and $foo2 are the same object, but you want a new object each time. Is this your problem?

There are 2 solutions:

  • The DI\factory definition doesn't allow to change the Scope -> it is fixable (it needs some code change though)
  • Use Container::make() as you said

I will have a look at the first solution: I'll try to add the option of "scope" to the "factory" definitions in the next release. It shouldn't be very hard. (i'll keep this issue open to track that new feature)

In the meantime, if you don't want to wait for that new feature, you can use make(). Like you said, you would need to inject the container into your objects, which is bad. Because of this, when I added the make method on the container, I also added a DI\FactoryInterface. The container implements that interface.

That interface allows to inject the container, without making your code directly coupled to the container. In short, your dependency is any factory. This would mean this configuration:

return [
    'DI\FactoryInterface' => DI\link('DI\Container');

    'Bar' => DI\object()
        ->constructor(DI\link('DI\FactoryInterface'));
];

So in your code you have:

class Bar
{
    private $foo;
    public function __construct(FactoryInterface $factory)
    {
        $this->foo = $factory->make('Foo');
    }
}

And notice there is nothing in your code that couples you to the container.

Member

mnapoli commented Jun 3, 2014

Hi! I think I see what you problem is, let me know if I'm wrong.

You have something like this:

return [
    'Foo' => DI\factory(function () {
        return new Foo();
    }),
];
$foo1 = $container->get('Foo');
$foo2 = $container->get('Foo');

Here $foo1 and $foo2 are the same object, but you want a new object each time. Is this your problem?

There are 2 solutions:

  • The DI\factory definition doesn't allow to change the Scope -> it is fixable (it needs some code change though)
  • Use Container::make() as you said

I will have a look at the first solution: I'll try to add the option of "scope" to the "factory" definitions in the next release. It shouldn't be very hard. (i'll keep this issue open to track that new feature)

In the meantime, if you don't want to wait for that new feature, you can use make(). Like you said, you would need to inject the container into your objects, which is bad. Because of this, when I added the make method on the container, I also added a DI\FactoryInterface. The container implements that interface.

That interface allows to inject the container, without making your code directly coupled to the container. In short, your dependency is any factory. This would mean this configuration:

return [
    'DI\FactoryInterface' => DI\link('DI\Container');

    'Bar' => DI\object()
        ->constructor(DI\link('DI\FactoryInterface'));
];

So in your code you have:

class Bar
{
    private $foo;
    public function __construct(FactoryInterface $factory)
    {
        $this->foo = $factory->make('Foo');
    }
}

And notice there is nothing in your code that couples you to the container.

@mnapoli mnapoli changed the title from Can injected object caching be disabled? to Prototype scope on "factory" definitions Jun 3, 2014

@mnapoli mnapoli added the enhancement label Jun 3, 2014

@mnapoli mnapoli added this to the 4.2 milestone Jun 3, 2014

@mnapoli mnapoli referenced this issue Jun 3, 2014

Merged

4.2 #165

11 of 11 tasks complete
@avant1

This comment has been minimized.

Show comment
Hide comment
@avant1

avant1 Jun 4, 2014

Contributor

Thanks for quick answer.

Here $foo1 and $foo2 are the same object, but you want a new object each time. Is this your problem?

Yes, everything is correct.

FactoryInterface is really very helpful.

Also I took a look to the source code. It turned out, that adding scope to FactoryDefenitionHelper is the only thing that needed to be done to enable scope
support for FactoryDefenition.

I've added this changes and it seems that everything works really well.
I think, I could be able to create a pull request, but I am not very familiar with github workflow.

Also, I have some troubles with running tests (got 21 fails).

Here is my commit for this fix: avant1@2dfa47c

Contributor

avant1 commented Jun 4, 2014

Thanks for quick answer.

Here $foo1 and $foo2 are the same object, but you want a new object each time. Is this your problem?

Yes, everything is correct.

FactoryInterface is really very helpful.

Also I took a look to the source code. It turned out, that adding scope to FactoryDefenitionHelper is the only thing that needed to be done to enable scope
support for FactoryDefenition.

I've added this changes and it seems that everything works really well.
I think, I could be able to create a pull request, but I am not very familiar with github workflow.

Also, I have some troubles with running tests (got 21 fails).

Here is my commit for this fix: avant1@2dfa47c

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Jun 4, 2014

Member

Hi, great! You should be able to create a pull request here: avant1/PHP-DI@mnapoli:4.2...4.2

We'll see in the pull request which tests are failing and how to fix it.

Member

mnapoli commented Jun 4, 2014

Hi, great! You should be able to create a pull request here: avant1/PHP-DI@mnapoli:4.2...4.2

We'll see in the pull request which tests are failing and how to fix it.

mnapoli added a commit that referenced this issue Jun 4, 2014

Merge pull request #167 from avant1/4.2
 #164 Prototype scope on "factory" definitions

mnapoli added a commit that referenced this issue Jun 4, 2014

@mnapoli mnapoli closed this Jun 4, 2014

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