[QueryBuilder] Allow to provide an "index" of Definitions?

[holtkamp]

When a lot of DIC Definitions are involved, I found it was useful to organize them in separate files, which contain arrays:

//Config1.php
return [
        LoggerInterface::class => \DI\create(Logger::class),
        //...
];

Then a single "index" file can be used to refer the these separate files:

//definitionsFiles.php
return [
    __DIR__ . \DIRECTORY_SEPARATOR . 'Config1.php',
    __DIR__ . \DIRECTORY_SEPARATOR . 'Config2.php',
    __DIR__ . \DIRECTORY_SEPARATOR . 'ConfigN.php',
];

Then this "index" of Definitions can be processed like this:

$definitionsFiles = require 'definitionsFiles.php';
foreach($definitionsFiles as $definitionsFile{
    $containerBuilder->addDefinitions($definitionsFile);
}

Would a PR with a function that accepts such an "index" of Definitions be accepted, or is out of of scope? Something like this:

//QueryBuilder.php
public function addDefinitionsIndex(string $definitionsIndex): self{
    foreach($definitionsIndex as $definitions{
        $containerBuilder->addDefinitions($definitions);
    }
}

[mnapoli]

I would rather like to keep the API as small as possible ("don't make me think"). Here we would just avoid a foreach so there doesn't seem to be a lot of added value.

I wonder though if addDefinitions() could be changed to take a list of files/arrays using variadic parameters. That way the API stays the same but it can support lists of definitions. Example of usage:

$builder->addDefinitions(...$definitionsIndex);

What do you think?

[holtkamp]

Aah, yeah, variadic parameters would be a great alternative to reach the same goal, nice!

[juliangut]

IMO the way you store your definitions and how you collect them should be out of the container

If you know you're not overriding any definition (DI\add or DI\decorate) you can just do this:

$builder->addDefinitions(\array_merge(...$definitionsIndex));

[holtkamp]

@juliangut note that in my example the $definitionsIndex is an array of strings (pointing to the configuration files) and not an array of arrays.

[mnapoli]

IMO the way you store your definitions and how you collect them should be out of the container

Agreed. Although if the variadic things work out that is a very little impact on the API (almost invisible) and easy to maintain so that's why I think "why not".

[juliangut]

@holtkamp that is right, I overlooked that part, anyway that is what I don't think fits the library, checking if passed a string and if so require the file, you can easily decorate builder to do so

@mnapoli just accepting a variadic list of definition arrays is a good addition, it would simplify this code to a single addDefinitions call

[mnapoli]

Done in #627!

[holtkamp]

Nice work @juliangut ! Thanks a bunch