contributte / di

:sparkles: Extra contrib to nette/di (@nette)
https://contributte.org/packages/contributte/di.html
MIT License
46 stars 9 forks source link

@Value annotatation discussion #5

Closed f3l1x closed 6 years ago

f3l1x commented 6 years ago

Come at here @enumag.

enumag commented 6 years ago

First let me ask. By the value objects you mentioned you mean something like WebConfiguration class with properties $projectDir, $tempDir, $publicDir and so on? And then wherever you need any of these parameters you would inject this class instead? Or did you mean something else?

f3l1x commented 6 years ago

Exactly. I mean for example MailConfig.

final class MailConfig
{

    /** @var string */
    private $from;

    public function getFrom(): string
    {
        return $this->from;
    }

    public function setFrom(string $from): void
    {
        $this->from = $from;
    }

}

With @value injection.

final class MailConfig
{

    /** @var string @value(%mail.from%) */
    private $from;

}
enumag commented 6 years ago

I don't quite see the benefit. These value objects are there to remove the need of manually injecting all the properties. You just inject them to this value object. But whether you do that in config or in the class itself doesn't make much of a difference - it's still about the same amount of writing. The downside here is that you're referencing the container parameters in your source code which is the same thing as $this->container->getParameter('whatever'). Imo the container parameter names should stay in config - same as all the service names which we're eliminating from source code by using DI instead of service locator.

And if you're doing it because you want to addsome validation for the properties as someone mentioned on twitter then I'm against that as well. I want to perform such validaiton at compile-time to both prevent doing useless things every time in runtime and to see the errors for things that are rarely used and I could miss them.

f3l1x commented 6 years ago

Hmmm, I get it.

There's a hidden purpose for the whole thing and to simplify configuration.

Consider you have a modular system and many configurations, you don't want to setup all neon files and their calls. You just want to share this value object and use it.

/** @var string @value(%mail.from%) */
private $from;

/** @var string @value(value="%mail.from%", type="email")  */
private $from;

/** @var string @value(value="%mail.from%", type="email", prefix="no-replay@")  */
private $from;

The last example is very futuristic, but I hope you know what I mean.

f3l1x commented 6 years ago

I'm closing it now, we can still discuss it. :-) Thanks anyway.

enumag commented 6 years ago

I missed your previous answer here, sorry.

Well I understand a bit better what is your intention but I still don't think it's a good idea and I would not personally use it. When you have a modular system it's more than likely that you'll need to let the user configure most of the parameters in admin interface in which case you probably won't have them as container parameters anyway.