Auto-wiring dependencies for setter injection about auryn HOT 22 CLOSED

I'm not @rdlowrey but I'm personally against this behavior. Every time I have personally wanted a setter method for a dependency it been because of poor program design.

My $0.02

from auryn.

@morrisonlevi Hey levi, I'm actually thinking controller-wise, if my url matches controller/action, and say my login controller requires a user service for the login action, and another service for a different action - why instantiate both dependencies when only one is used for each method? Is that poor program design by mapping url to controller / action?

from auryn.

If it is a different action that requires different dependencies, then I would say it is bad design. The alternative option is to pass the dependency needed for just that action at method call time:

    $controller->action($id, $param, $extraDependencyForAction)

I'd like to clarify that this is just based off my experience. I could be wrong here, but I wanted to voice my opinion on the matter.

from auryn.

I generally prefer constructor injection over setter injection as well. However, I have in the past used Auryn in conjunction with a custom router class to (1) provision a "controller" class AND (2) subsequently provision the parameters for the controller's "action" method using a combination of arguments parsed from a URI and definitions for parameter typehints. While it's not the same thing as setter injection, it's not too far off. I think the better course of action may be to expand the ReflectionStorage interface so that it can readily cache function and parameter list reflections for methods that aren't __construct. This would make it relatively straightforward to implement your own flavor of automatic setter injection by doing something like the following hypothetical:

class MyRouter {

    private $injector;
    private $reflStorage;

    function __construct(Auryn\Injector $injector, Auryn\ReflectionStorage $reflStorage) {
        $this->injector = $injector;
        $this->reflStorage = $reflStorage;
    }

    function route($method, $uri) {
        // ... do some routing to get $className, $methodName and $uriArgs parsed from $uri ... //

        $controller = $this->injector->make($className);
        $reflMethod = $this->reflStorage->getMethod($className, $methodName); // hypothetical, not implemented
        $reflParams = $this->reflStorage->getMethodParams($reflMethod); // hypothetical, not implemented

        $mergedArgs = [];

        foreach ($reflParams as $reflParam) {
            $paramName = $reflParam->name;
            if (array_key_exists($paramName, $uriArgs)) {
                $mergedArgs[$paramName] = $uriArgs[$paramName];
            } elseif ($typehint = $this->reflStorage->getParamTypeHint($reflMethod, $reflParam)) {
                $mergedArgs[$paramName] = $this->injector->make($typehint);
            } elseif ($reflParam->isDefaultValueAvailable()) {
                $mergedArgs[$paramName] = $reflParam->getDefaultValue();
            } else {
                $mergedArgs[$paramName] = NULL;
            }
        }

        $reflMethod->invokeArgs($controller, $mergedArgs);
    }
}

from auryn.

I know that setter injection is used for optional dependencies in the class - and I thought it was pretty common.

Auryn already uses $injector->make($controller); which instantiates required dependencies.

I was just hoping for it to also instantiate dependencies depending on the $action being called too.

Running $controller->action($id, $param, $extraDependencyForAction) is not automatically wired (I'd need to compose a graph for every single optional one). Although if doing something like this would inspire bad architecture, I suppose it would be for the best leaving it out.

from auryn.

Note that you can implement something like the code above right now. You just can't use the reflection storage instance to cache the method and parameter typehints with the current code. You'd simply need to replace the two hypothetical calls to ReflectionStorage::getMethod and ReflectionStorage::getMethodParams and retrieve those reflections directly using PHP's reflection API.

from auryn.

@J7mbo Is this still something you're interested in? I'm kicking around the idea of adding an Injector::execute method that accepts a callable (or an array [$className, $methodName] that can be provisioned automatically) and an optional array of values. So the API would look something like this:

class HelloDependency {
    function saySomething() { return 'Hello'; }
}

class Greeting {
    function greet(HelloDependency $hello, $name) {
        echo $hello->saySomething() . ', ' . $name;
    }
}

// Will echo "Hello, world"
$provider->execute(['Greeting', 'greet'], $args = [':name' => 'world']);

This would accomplish what you're asking and would eliminate some steps when the ultimate goal is to actually do something with the objects you instantiate.

from auryn.

@rdlowrey Definitely still interested in having this added: can the constructor injection still happen automatically for class-based dependencies and then, when running your $provider-execute() method, have the method dependencies recursively instantiated? If so, 👍 !

from auryn.

Yeah that's the plan. Alternatively you could pass in a preexisting instance and only the relevant method's parameters would be provisioned:

$provider->execute([$existingGreetingObj, 'greet'], $args = [':name', 'world']);

It would also work for any other valid callable. For example:

$myGreeter = function(HelloDependency $dep, $name) {
    echo $dep->saySomething() . ', '. $name;
};
$provider->execute($myGreeter, $args = [':name' => 'world']);

I'll put this on the menu to work on. I may get around to it today but we'll just see how things go. Regardless I'm likely to implement it soon.

from auryn.

Looks awesome! Looking forward to it :)

from auryn.

@J7mbo I've just committed the new Injector::execute() functionality. You can now execute any valid PHP callable with full recursive provisioning of its method signature:

$injector->execute(function(){});
$injector->execute([$objectInstance, 'methodName']);
$injector->execute('globalFunctionName');
$injector->execute('MyStaticClass::myStaticMethod');
$injector->execute(['MyStaticClass', 'myStaticMethod']);
$injector->execute(['MyChildStaticClass', 'parent::myStaticMethod']);
$injector->execute('ClassThatHasMagicInvoke');
$injector->execute($instanceOfClassThatHasMagicInvoke);

Additionally, you can pass in the name of a class for a non-static method and the injector will automatically provision an instance of the class (subject to any definitions or shared instances already stored by the injector) before provisioning and invoking the specified method:

class Dependency {}
class AnotherDependency {}
class Example {
    function __construct(Dependency $dep){}
    function myMethod(AnotherDependency $adep, $arg) {
        return $arg;
    }
}

$injector = new Auryn\Provider;
var_dump($injector->execute(['Example', 'myMethod'], $args = [':arg' => 42]));
// outputs: int(42)

In conclusion, please test this and let me know if you have any questions/comments/feedback. It's very well unit-tested already so it should be bug-free but you never know. If everything seems cool in a couple of days I'll go ahead and tag a new release.

from auryn.

Woops -- looks like I forgot to add support for classes with a magic __invoke() method. I've just committed, tested and pushed the additional Injector::execute() functionality for invokable objects.

from auryn.

I've been playing around with the new code this evening, and so far it's totally brilliant! Can't fault it, method injection for Controller / Action works perfectly. :D

from auryn.

Cool. Thanks for the feedback. And thanks or the initial suggestion -- it took me awhile to get over my initial misgivings but I think it's a nice feature at this point.

from auryn.

This is really cool. That is all. :)

from auryn.

@ascii-soup Thanks; hopefully people find the feature useful.

from auryn.

Just stumbled upon this problem here:

Beforehand I had an instance of a controller a

$controller = $injector->make($controllerClass);

and called it like

$result = call_user_func_array(array($controller, $method), $args);

That worked great (I could pass any number of any params as $args).

Now I tried to make that possible using ->execute(), but that requires passing all the arguments manually filling the array.

So how may I pass all arguments plus injecting the ones typehinted?

I think that way controller method would look like this:

public function dosomething($id, $name, \App\Repository\Users $users)
{
}

That looks possible only with the 'raw named parameter', but not unnamed. Forcing all controller actions to use named params like $arg1, $arg2 etc. is definitely not worth it.

I hope you understand my issue, maybe it's a poor design, but I don't think this is the case

from auryn.

Are the $id and $name parameters parameters passed in from your GET / POST request (as in slugs)?

from auryn.

Exactly, so they're not constant, there may be one, more or none

from auryn.

I see that I may do that like (IIRC) it was in codeigniter using something like ->segment(n), but that's ugly :)

from auryn.

Get your $params as an array from your request, then prefix them with a : for scalar injection with Auryn:

    array_walk($params, function($value, $key) use (&$args) {
        $args[sprintf(':%s', $key)] = $value;
    });

This is how it works for my in Symfony, but you can customise for yourself. See how I'm making the controller, then executing the action with the given $args (the : params).

    /** Executed by the HTTP Kernel **/
    return function () use ($controller, $action, $args) {
        return $this->injector->execute([$this->injector->make($controller), $action], $args);
    };

from auryn.

@J7mbo, thank you, works great, the only downside is that I should name the arguments the same way as they're named in $params, which comes from nikic/FastRoute, but that makes the code even cleaner by forcing to stick to same names!

from auryn.