auraphp / Aura.Di

Dependency Injection System
MIT License
349 stars 63 forks source link

Ability to have more than one argument for setters #104

Closed acim closed 8 years ago

acim commented 9 years ago

At the moment it is possible to assign just one value to each setter, but it would be nice to have possibility to insert more. In my case I wanted to configure Spot 2 ORM which had a setter with two arguments. I had to override this class and use PHP 5.6 variadic, but there are maybe some other solutions. Of maybe check if version is >= 5.6 use variadic, otherwise use some other trick.

acim commented 9 years ago

So, to clear the things up, I mean Spot\Config class, addConfiguration setter which takes two parameters. In order to solve this, I passed array by Aura.Di and overrode Spot\Config constructor with 5.6 variadic feature. I was lucky we are already using 5.6 :)

https://github.com/vlucas/spot2

There are probably many other examples.

acim commented 9 years ago

I will try to arrange this on the other side ;)

harikt commented 9 years ago

If I understand you correctly you were trying to add different connections via addConnection https://github.com/vlucas/spot2/blob/403f7812000f723f3f372c730b4612d16f78aec1/lib/Config.php#L24 .

acim commented 9 years ago

I am doing this:

use MyNamespace\SpotConfig as Config; $container->setters[Config::class]['addConnection'] = ['mysql', $dbConfig]; $container->params[Locator::class]['config'] = $container->lazyNew(Config::class); $container->set('spot_orm', $container->lazyNew(Locator::class));

But in order to pass this array ['mysql', $dbConfig] as two arguments, I had to do this:

namespace MyNamespace; class SpotConfig extends Config { public function addConnection(array $config) { return parent::addConnection(...$config); } }

Luckily, we run 5.6 so I could do this.

By the way, Locator and Config classes are from Spot 2 ORM.

https://github.com/vlucas/spot2

harikt commented 9 years ago

@acim Thanks for sharing what you are doing.

I don't recall, but if I am correct the below should work.

$container->setters[Config::class]['addConnection'] = [ 'name' => 'mysql', 'dsn => $dbConfig];

This is how params work, and my assumption is setters will be working similar.

acim commented 9 years ago

Unfortunately this doesn't work, I have already tried and debugged.

<?php
use Aura\Di\ContainerBuilder;

require_once dirname(__DIR__) . '/vendor/autoload.php';

class Test
{
    public function setMe($param1, $param2)
    {
        echo "$param1 $param2";
    }
}

$builder = new ContainerBuilder();
$di = $builder->newInstance();

$di->setters['Test']['setMe'] = ['param1' => 1, 'param2' => 2];
$di->set('test', $di->newInstance('Test'));

$test = $di->get('test');

$di->setters['Test']['setMe']['param1'] = 1;
$di->setters['Test']['setMe']['param2'] = 2;
$di->set('test2', $di->newInstance('Test'));

$test2 = $di->get('test2');
harikt commented 9 years ago

Thank you.

silentworks commented 9 years ago

The issue is Aura doesn't resolve anything for setters, it just passes the first value only. So its not doing like what it does for a class constructor where it will look at the params then match them by name. I think this is an expected behaviour, seeing as setters are normally portrayed as bad, you can still do this however by using the solution I provided on the issue on the Spot project.

harikt commented 9 years ago

:+1: .

acim commented 9 years ago

Bad or not, some (lot of) setters are using multiple parameters and it would be convinient to support this in an easy way. This functionality is already present for constructors, reflection is already there, so... :)

Aura is too cool not to support such simple thing. But I will continue using it anyway :)

pmjones commented 9 years ago

Well, 3.x would be the place to make it happen, since it's not stable yet. Let me see what I can do. :-)

acim commented 9 years ago

By the way, I am using v3 :)

pmjones commented 9 years ago

Perfect!

pmjones commented 8 years ago

All right @acim, check out the new multiset branch, which allows you to make calls to multi-argument setter methods; see the notes in this commit: https://github.com/auraphp/Aura.Di/commit/4d322510aa6f3048d6ef3af73e444eab2791e961

In particular, you have to pass an array of values to $di->methods; these will be used with call_user_func_array().

Also, just like with single-value setter methods, it will only make the method call once. If you need to call the same method multiple times, this will not work for you. (It was only after I did the work that I realized that your use case with Spot2 might need multiple calls to addConnection(), so I need to point that out.)

Honestly, I'm not really thrilled by this functionality, but if you think it actually meets your current and future needs, I'm happy to merge it back to 3.x. However, if you have any doubts at all, or have come up with a better solution for your use case, please let me know as soon as you can.

acim commented 8 years ago

Thank you @pmjones, this is exactly what I have needed. However, I already solved it this way:

use Spot\Config as SpotConfig;
use Spot\Locator;

        $di->set('spot_config', $di->lazy(function () use ($dbConfig) {
            $spotConfig = new SpotConfig();
            $spotConfig->addConnection('mysql', $dbConfig, true);
            return $spotConfig;
        }));
        $di->params[Locator::class]['config'] = $di->get('spot_config');
        $di->set('spot_orm', $di->lazyNew(Locator::class));

If you feel this new code makes some problems, you can just drop it. Otherwise merge it, so this is up to you. Thank you once more for your effort.

pmjones commented 8 years ago

@acim Thanks for following up. With your reply in mind, I will not merge the new branch, and will leave setters as single-value only. That will put the library on track to an earlier stable release. Cheers!