auraphp / Aura.Di

Dependency Injection System
MIT License
349 stars 63 forks source link

Lazy Factory Call #187

Closed frederikbosch closed 5 years ago

frederikbosch commented 5 years ago

There is one feature missing in my opinion lazyFactoryCall.

$container->set('money.formatter.factory', $container->lazyNew(MoneyFormatterFactory::class));
$container->set('money.formatter', $container->lazyFactoryCall('money.formatter.factory'));

The same result can be achieved by doing this, but I don't like it because it is not explicit enough.

$container->set('money.formatter.factory', $container->lazyNew(MoneyFormatterFactory::class));
$container->set('money.formatter', $container->lazyGetCall('money.formatter.factory', '__invoke'));

The function would be really easy to implement because it would be a specialization of lazyGetCall. @harikt What do you think?

harikt commented 5 years ago

Hey @frederikbosch ,

I am not using the library for some time now. It is not because I don't like it, but my work is on a different one. So sometimes my answer for your question may be in complete or not what you are looking for. So consider my ignorance.

Regarding your question : I don't think the Factory class may not be always having __invoke , but instead some other methods also. So do you think if you alter the below one it will also work ?

$container->set('money.formatter', $container->lazyFactoryCall('money.formatter.factory'));

Eg:

$container->set('money.formatter', $container->lazyFactoryCall('money.formatter.factory', '__invoke'));
$container->set('money.formatter', $container->lazyFactoryCall('money.formatter.factory', 'newFacory'));

I am looking at https://github.com/Sylius/Product/blob/69d9f1513aa62b203676f8ed243653009c98cff8/Factory/ProductFactory.php#L38-L41

What is your take ?

Else I will stick on to the lazyGetCall .

frederikbosch commented 5 years ago

You are right. Let's stick with lazyGetCall. In my opinion we can tag a first beta. I have been using the alpha for quite a while now. No issues at all.

harikt commented 5 years ago

@frederikbosch Good to hear. Keep rocking :+1: .

frederikbosch commented 5 years ago

Thanks, shall I tag the first beta?

harikt commented 5 years ago

why not ?

Go ahead.

frederikbosch commented 5 years ago

Done.

harikt commented 5 years ago

Congrats! :+1: .

frederikbosch commented 5 years ago

@harikt If you ask me we can tag 4.0.0. I did not find any bugs while I am using the package heavily. And to be honest: I need it to be on stable version to prevent all kinds of composer workarounds. Would you agree on tagging 4.0.0?

harikt commented 5 years ago

Why not ? We can always fix / push new releases.