auryn icon indicating copy to clipboard operation
auryn copied to clipboard

Lazy instantiation

Open tolik89u opened this issue 9 years ago • 22 comments

Is it possible to lazily instantiate my class with Auryn?

I know, there is method Auryn\Injector::delegate, which allows to defer instantiation, but inside its parameter factory closure it is impossible to call Auryn's instantiating method Auryn\Injector::make to instantiate this object. If I do so, injector throws an exception: «Uncaught Auryn\InjectionException: Detected a cyclic dependency while provisioning MyComplexClass» — obviously, because is a cyclic dependency:

class MyComplexClass {
    public $verification = false;
    public function doSomethingAfterInstantiation() {
        $this->verification = true;
    }
}

$injector = new Auryn\Injector;

$complexClassFactory = function() use($injector) {
    $obj = $injector->make('MyComplexClass');
    $obj->doSomethingAfterInstantiation();

    return $obj;
};

$injector->delegate('MyComplexClass', $complexClassFactory);

$obj = $injector->make('MyComplexClass');

So, how do I lazily instantiate my object using Auryn's injector, without having to manually specify the code which instantiates the object?

tolik89u avatar Apr 04 '16 15:04 tolik89u

you shouldnt pass arround the injector (Service Locator anti pattern) and your class should not rely on a method beeing invoked after instantiation.. why dont you just use __construct()?

staabm avatar Apr 04 '16 15:04 staabm

@staabm, I don't talk about using auryn as service locator, but I'm talkin about dependency injection.

I've already understood that calling Auryn\Injector::make() inside of Auryn\Injector::delegate() wouldn't have any effect, because instance would be created in the same time when it has been injected, and whould not be delayed until first time the object is used.

But nevertheless, I think that the feature of lazy instantiation is awesome, and it would be really cool if auryn could do that. For example, PHP-DI has such a feature: https://github.com/PHP-DI/PHP-DI/blob/master/doc/lazy-injection.md

Here's a simple example, in which class Dependency is injected into MyClass, using PHP-DI. Note that it is injected, not «service-located»:

<?php
require $_SERVER['DOCUMENT_ROOT'] . '/vendor/autoload.php';

class Dependency
{
    public function __construct()
    {
        echo 'Dependency has been initialized' . PHP_EOL;
    }
    public function doSomething()
    {
    }
}

class MyClass
{
    public $dependency;

    public function __construct(Dependency $dependency)
    {
        $this->dependency = $dependency;
    }
}

echo '<pre>';

echo 'Here is how it can be done using PHP-DI.' . PHP_EOL;

//bootstrapping PHP-DI
$builder = new DI\ContainerBuilder();
$container = $builder->build();
$container->set(Dependency::class, \DI\object()->lazy());

$myClassInstance = $container->get(MyClass::class);
// $dependency is now a Proxy object, it is not initialized, it is very lightweight in memory
$dependency = $myClassInstance->dependency;
var_dump($myClassInstance instanceof MyClass); // true
var_dump($dependency instanceof Dependency); // true
$dependency->doSomething(); // outputs: Dependency has been initialized
var_dump($dependency instanceof Dependency); // it's still true, of course
echo PHP_EOL;


echo 'And this is how it could be done using Auryn.' . PHP_EOL;

//bootstrapping Auryn
$injector = new Auryn\Injector;
$injector->defineLazyProxy(Dependency::class);//this method doesn't exist yet

$myClassInstance = $injector->make(MyClass::class);
$dependency = $myClassInstance->dependency;
var_dump($myClassInstance instanceof MyClass);
var_dump($dependency instanceof Dependency);
$dependency->doSomething();
var_dump($dependency instanceof Dependency);
echo PHP_EOL;

tolik89u avatar Apr 04 '16 17:04 tolik89u

This comment confused me: https://github.com/rdlowrey/Auryn/issues/77#issuecomment-76518160 It's said in it:

Auryn allows you to use factories for lazy instantiation

— that's why I decided incorrectly, that lazy instantiation is already possible in auryn using delegate().

tolik89u avatar Apr 04 '16 17:04 tolik89u

I've just implemented lazy inititialization using Auryn and Ocramius/ProxyManager — ProxyManager is the library, which is used by PHP-DI to create lazy proxies: https://github.com/Ocramius/ProxyManager/blob/master/docs/lazy-loading-value-holder.md

Here I post the updated code:

<?php
use ProxyManager\Factory\LazyLoadingValueHolderFactory;
use ProxyManager\Proxy\LazyLoadingInterface;

require $_SERVER['DOCUMENT_ROOT'] . '/vendor/autoload.php';

class Dependency
{
    public function __construct()
    {
        echo 'Dependency has been initialized' . PHP_EOL;
    }
    public function doSomething()
    {
    }
}

class MyClass
{
    public $dependency;

    public function __construct(Dependency $dependency)
    {
        $this->dependency = $dependency;
    }
}

echo '<pre>';

echo 'Here is how it can be done in PHP-DI.' . PHP_EOL;

//bootstrapping PHP-DI
$builder = new DI\ContainerBuilder();
$container = $builder->build();
$container->set(Dependency::class, \DI\object()->lazy());

$myClassInstance = $container->get(MyClass::class);
// $dependency is now a Proxy object, it is not initialized, it is very lightweight in memory
$dependency = $myClassInstance->dependency;
var_dump($myClassInstance instanceof MyClass); // true
var_dump($dependency instanceof Dependency); // true
$dependency->doSomething(); // outputs: Dependency has been initialized
var_dump($dependency instanceof Dependency); // it's still true, of course
echo PHP_EOL;


echo 'And this is how it could be done in Auryn.' . PHP_EOL;

//bootstrapping Auryn
$injector = new Auryn\Injector;
$injector->delegate(Dependency::class, function() { 
    $factory = new LazyLoadingValueHolderFactory();
    $initializer = function (& $wrappedObject, LazyLoadingInterface $proxy, $method, array $parameters, & $initializer) {
        $initializer   = null; // disable initialization
        $wrappedObject = new Dependency(); // fill your object with values here
        return true; // confirm that initialization occurred correctly
    };
    return $factory->createProxy(Dependency::class, $initializer); 
});

$myClassInstance = $injector->make(MyClass::class);
$dependency = $myClassInstance->dependency;
var_dump($myClassInstance instanceof MyClass); // true
var_dump($dependency instanceof Dependency); // true
$dependency->doSomething(); // outputs: Dependency has been initialized
var_dump($dependency instanceof Dependency); // it's still true, of course
echo PHP_EOL;

And here is what it outputs:

Here is how it can be done in PHP-DI.
bool(true)
bool(true)
Dependency has been initialized
bool(true)

And this is how it could be done in Auryn.
bool(true)
bool(true)
Dependency has been initialized
bool(true)

tolik89u avatar Apr 04 '16 17:04 tolik89u

What helped me write better code with automatic DI was to think beforehand about how would I do the exact same thing without Auryn. Auryn is nor a framework, nor a pattern, you basically write your code ready for DI and then "apply" Auryn instantiation that removes your instantiation calls (new ...). If you're trying to do it otherwise, you're probably doing it wrong.

vlakarados avatar Apr 05 '16 03:04 vlakarados

I think we're going off topic a little. The issue here is that:

$complexClassFactory = function() use($injector) {
    $obj = $injector->make('MyComplexClass');
    $obj->doSomethingAfterInstantiation();

    return $obj;
};

$injector->delegate('MyComplexClass', $complexClassFactory);

Calling make() within a delegate() causes the cyclic dependency issue because the same class is requiring a factory to create it. You're saying to the injector:

  • When we encounter MyComplexClass, use a factory to create it
  • The factory starts up, and then sees a make, which goes back to the factory you are delegating etc..

This is your own code :P

J7mbo avatar Apr 05 '16 09:04 J7mbo

@vlakarados, I'm not sure if I correctly understood you. When you say:

and then "apply" Auryn instantiation that removes your instantiation calls (new ...). If you're trying to do it otherwise, you're probably doing it wrong.

— do you mean that when I ask auryn to instantiate my objects — it's OK, but when I ask it to do it lazily — this is not OK, this is wrong — did I correctly understand you?

I don't want to immediately instantiate ALL my heavy-complex-objects, which take too much time to initialize, and I don't wanna init them on every request, directly in bootstrap.php. Can I do so that auryn instantiates it by demand, only when it's really necessary, and not on every single hit?

Or do you suggest that I use factories instead of making auryn create proxies for me? In this case, how should I share these objects, produced by those factories? Should I use singletons? — that's an anti-pattern I don't want to use... So, what solution of this problem do you propose? Please, explain.

tolik89u avatar Apr 05 '16 12:04 tolik89u

@J7mbo , the real topic here is lazy instantiation using auryn. As I said in first post of this discussion:

how do I lazily instantiate my object using Auryn's injector, without having to manually specify the code which instantiates the object?

You wrote:

Calling make() within a delegate() causes the cyclic dependency ... This is your own code :P

Yes, I understand it. These are also my own words:

obviously, because is a cyclic dependency: ... I've already understood that calling Auryn\Injector::make() inside of Auryn\Injector::delegate() wouldn't have any effect, because instance would be created in the same time when it has been injected, and whould not be delayed until first time the object is used.

So, yes, I understand this. The real topic is: lazy instantiation using auryn. I've concluded that auryn doesn't have such a feature, so I propose to add it (this issue becomes a feature request). I even have already provided the code that works and lazily instantiates injected dependencies using auryn and ProxyManager — this code could be used as a basis to implement this feature.

tolik89u avatar Apr 05 '16 12:04 tolik89u

@tolik89u sorry, it was a misunderstanding on my side..

vlakarados avatar Apr 05 '16 12:04 vlakarados

@tolik89u The best solution to your problem has nothing to do with Auryn. Refactor your class to not do all that work in the constructor. Please read this excellent article: http://misko.hevery.com/code-reviewers-guide/flaw-constructor-does-real-work/

garrettw avatar Apr 07 '16 21:04 garrettw

@garrettw, thanks for the article, it was useful to read. I completely agree with what is written there: that

  • Object should have a single responsibility and shouldn't initialize its dependencies, but should get it from those who invoke it.
  • Object should perform all needed work to initilize itself in the constructor, and not to move it to some init method — so that it never be in an inconsistent, invalid state.

But this doesn't solve my problem. In any case, if the object is instantiated (and has all of its dependencies injected into and they're also already instantiated) — that means that a lot of work already has been done to init this object and all of its dependencies. And no matter, whether this work is done inside this object's constructor, or the injector has instantiated its dependencies and they did all necessary work in their constructors respectively. The fact is that this work is already done.

My proposition is to delay all this work until it will be needed. Because there are some cases when this dependency could be never used — so in these cases, why to perform all that work?

tolik89u avatar Apr 11 '16 09:04 tolik89u

My point is that instantiating dependencies shouldn't be that much work if their constructors aren't doing anything but assigning stuff to properties.

garrettw avatar Apr 11 '16 13:04 garrettw

@garrettw , if ALL dependencies of the object will be fully initialized before this object's constructor, and than they're injected into its constructor, and then the object itself is be initialized — than ALL the initialization work will be done. If you skip some initialization work — than it means that some of your dependencies were initialized only partially, which is wrong, because now they are in an inconcistent, invalid state, and dependent classes have to compete its initialization — this is wrong. This is what I'm saying about: if there is some “heavy” classes — they will be heavy — no matter whether they perform initialization of its dependencies in the constructor — or the injector performs this initialization before injection. And deferring initialization is wrong. This is why people invented patterns like “value holder” (lazy loading). If this pattern is used, then none of the initialization work is performed until it's really needed. And at the same time the dependant objects (clients) don't need to manually initialize their dependencies (services). It'd be good if auryn also could benefit from using this pattern.

tolik89u avatar Apr 11 '16 16:04 tolik89u

Check out this library I just found. It looks like it can add that functionality to any DIC, if I'm not mistaken. https://github.com/snapshotpl/lazy-container

garrettw avatar Apr 13 '16 14:04 garrettw

If this isn't how it's meant to be done, how do you create new instances of classes? I don't understand this system at all!

MrBrax avatar Oct 25 '17 18:10 MrBrax

Hi, because I needed this feature for working with events in WordPress stuff I just implemented the ProxyManager in a forked version of the Auryn\Injector you can find here Auryn with Proxy Manager, now it can be possible to declare what class should be proxied with a dedicated method $injector->proxy(MyHeavyClass::class) and then call $injector->make(MyHeavyClass::class) to have a proxied object, it works also for every dependency, for example with the code posted initially:

class Dependency
{
    public function __construct()
    {
        echo 'Dependency has been initialized' . PHP_EOL;
    }
    public function doSomething()
    {
    }
}

class MyClass
{
    public $dependency;

    public function __construct(Dependency $dependency)
    {
        $this->dependency = $dependency;
    }
}

$injector->proxy(Dependency::class); // This will be proxied
$my_class = $injector->make(MyClass::class);

// It works as expected and $my_class->dependency will be replaced with the original one only when a call happens
$my_class->dependency->doSomething();

Auryn still works as expected, all tests passes with the tests for proxy I added.

Here you can find an implementation with this functionality Empress where I copyed the class I modified inside the forked version (because all methods are private) and then I extend the the original Auryn.

The only problem is that the Proxy Manager version I used (2.2) works only with PHP 7.2 >= and Auryn 1.4 is PHP 5.3 >=, merging in the base project it isn't possible for BC, if the methods inside the Auryn project were protected instead of private it would have been possible to create an adapter without copiing and paste code.

In case this project need help I am available, I don't want to mantain a forked version of it (it all depends on the future of this project).

overclokk avatar Jan 31 '20 16:01 overclokk

if the methods inside the Auryn project were protected instead of private it would have been possible

@MrBrax Was there a particular method that was the problem? Or just most of the stateful private ones...

In my opinion, Auryn is meant to be extendible so that people can sub-class it to add custom functionality, rather than having to maintain forks or complete replacements.

Off the top of my head what would need to be done to support this 'properly' would be for:

i) Auryn to define an interface for the proxy method, which this library wouldn't implement, but would be a standard interface so that people can use different proxy implementations.

ii) Have a separate project that extends the injector class with the proxy method implemented, probably using the ocramius/proxy-manager first, and have that other project deal with the composer issues coupling auryn to proxy-manager.

That's about it?

This seems like reasonable amount of stuff that could be done...

Danack avatar Jan 31 '20 17:01 Danack

In my opinion, Auryn is meant to be extendible so that people can sub-class it to add custom functionality, rather than having to maintain forks or complete replacements.

The problem is that actually it can't be possible to change the internal behavior for example for the make() method because the methods and properties used here are private, I can't call them on the child make(), it can be only possible to do some decoration calling the parent::make().

Here an example of the changes I made in the make() method:

if (isset($this->delegates[$normalizedClass])) {
	$executable = $this->buildExecutable($this->delegates[$normalizedClass]);
	$reflectionFunction = $executable->getCallableReflection();
	$args = $this->provisionFuncArgs($reflectionFunction, $args, null, $className);
	$obj = call_user_func_array(array($executable, '__invoke'), $args);
} elseif (isset($this->proxies[$normalizedClass])) {
	if ( isset( $this->prepares[$normalizedClass] ) ) {
		$this->prepares_proxy[$normalizedClass] = $this->prepares[$normalizedClass];
	}
	$obj = $this->resolveProxy($className, $normalizedClass, $args);
	unset($this->prepares[$normalizedClass]);
} else {
	$obj = $this->provisionInstance($className, $normalizedClass, $args);
}

As you can see I had to change the if else statement and also check inside the $this->prepares property.

Inside the proxy generator I used $this->provisionInstance() and $this->prepareInstance() to do the job in the lazy instance.

The proxy() method where I store the classes that need to be proxied I used $this->resolveAlias()

Here the code I wrote: https://github.com/ItalyStrap/empress/blob/a21c863b64a3dc21060705c51a001e0dfb071050/bridge/Injector.php#L312 https://github.com/ItalyStrap/empress/blob/a21c863b64a3dc21060705c51a001e0dfb071050/bridge/Injector.php#L339 https://github.com/ItalyStrap/empress/blob/a21c863b64a3dc21060705c51a001e0dfb071050/bridge/Injector.php#L398

i) Auryn to define an interface for the proxy method, which this library wouldn't implement, but would be a standard interface so that people can use different proxy implementations.

Yes, this would be nice and it would also be nice to have an InjectorInterface with all public methods.

ii) Have a separate project that extends the injector class with the proxy method implemented, probably using the ocramius/proxy-manager first, and have that other project deal with the composer issues coupling auryn to proxy-manager.

Yes, this is also nice but only if it would be possible to change the internal behavior of the Injector.

overclokk avatar Jan 31 '20 18:01 overclokk

Other than it feeling like too much code is being copied, there doesn't appear to be a technical reason against having the child class re-implement the whole of the make method.

Probably would be good if the whole of the Auryn test suite could be re-run against the child class, to make sure that copied implementation performs the same for all other cases...

Yes, this would be nice and it would also be nice to have an InjectorInterface with all public methods.

Can you say explicitly why this would be good, other than just in general terms? I'm not strongly against it, just prefer discussions to not be based on assumptions...

Danack avatar Jan 31 '20 18:01 Danack

Probably would be good if the whole of the Auryn test suite could be re-run against the child class, to make sure that copied implementation performs the same for all other cases...

All tests was executed against the Injector I modified, I have also activated a travis build in my account https://travis-ci.org/overclokk/auryn (it passes only test agains 7.2 and 7.3 of PHP obviously), now I just created a new branch Child were I created a Child class and a ChildTest and reimplemented the original Injector and InjectorTest for testing the behavior if some methods are removed and the tests fails obviously.

Can you say explicitly why this would be good, other than just in general terms? I'm not strongly against it, just prefer discussions to not be based on assumptions...

At this time it is only for general terms and for personal preference.

overclokk avatar Jan 31 '20 20:01 overclokk

Your PR is welcome, I don't have an issue with requiring a newer version of PHP, it's not even a BC break. Technical details can then be discussed on the PR.

kelunik avatar Jan 31 '20 23:01 kelunik

Ok, I'll clean up some code and I'll push a PR.

overclokk avatar Feb 02 '20 17:02 overclokk

I know it's been less than two years, but I think I'm going to close this issue rather than chase for a PR.

I added some words in d730160de423923fd38b2dda550fdee4b296e3f9 to let people know that the fork with lazy instantiation exists, and also some words as to why I think it probably shouldn't be added as a feature.

Danack avatar Dec 21 '22 16:12 Danack