Closed xepozz closed 4 years ago
Hi Dmitriy, Have a look here: https://github.com/container-interop/container-interop/issues/50 Having the service be aware of a container is a bad practice: https://www.php-fig.org/psr/psr-11/meta/#4-recommended-usage-container-psr-and-the-service-locator
Of course it's anti-pattern, but it will help to implement lazy loading and provide to end users clear constructor, lightweight initialization and less using resources (cpu initialize not used dependencies and memory store it)
Having a tool does not oblige you to use it. Let's see some cases:
For example, vendor provide some AbstractService
:
abstract class AbstractService
{
public function __construct(VendorDep1 $dep1, VendorDep2 $dep2)
{
// ....
}
}
If end user want to use it he need to resolve his own and vendor's dependencies
class Service extends AbstractService
{
public function __construct(UserDep1 $dep1, UserDep2 $dep2, VendorDep1 $vendorDep1, VendorDep2 $vendorDep2)
{
// ....
}
public function someAction(UserDep3 $dep3)
{
return $this->someMethod(/* ... */);
}
}
Vendor using lazy loading to resolve own dependencies. The constructor is absent
abstract class AbstractService
{
use ContainerAwareTrait;
public function someMethod(...$args)
{
$actor = $this->container->get(ActorInterface);
return $actor->do($args);
}
}
User declaring only own dependencies.
class Service extends AbstractService
{
public function __construct(UserDep1 $dep1, UserDep2 $dep2)
{
// ....
}
public function someAction(UserDep3 $dep3)
{
return $this->someMethod(/* ... */);
}
}
As we see the user's Service
haven't any redundant dependencties and the interface of AbstractService::someMethod
remained as it was.
If user won't call the vendor method someMethod
he won't resolve all vendor dependencies.
Imagine a lot of handlers with too hard dependencies. Need to match only one handler from the stack to process it further.
$handlers = [/* 10k handlers */];
$handler = $this->handlerMatcher->match($request, $handlers);
$response = $handler->handle($request);
In this case we have 10k classes and theirs resolved dependencies, but we use only one. The effectiveness of this approach tends to zero.
$handlers = [/* 10k handlers */];
$handler = $this->handlerMatcher->match($request, $handlers);
$handler->setContainer($container);
$response = $handler->handle($request);
In this case we have 10k classes and 0 resolved dependencies before handling. The effectiveness of this approach tends to max.
Any tool may very usefull if you know how to cook it.
Upd:
We have already complete LoggerAwareInterface
and some example for a factories with using the container as a service locator. I don't get your point.
Having the container injected is a bad practice:
Your comments do not make sense when it comes to increasing productivity in really large systems. Covering unit tests with such classes is not difficult. Hiding explicit dependencies is not a problem that I propose to solve.
In addition, I repeat, the presence of a tool does not oblige you to use it.
Your comments do not make sense when it comes to increasing productivity in really large systems. Covering unit tests with such classes is not difficult.
Unit testing will not be enough when you start injecting the whole container just to lazily reach to your needed dependencies instead of injecting them. You would be taking a shortcut, which would bite you back later.
Hiding explicit dependencies is not a problem that I propose to solve.
It's a problem that you're creating with this interface.
In addition, I repeat, the presence of a tool does not oblige you to use it.
It's a foot-gun. It's a service locator. It hinders unit testing.
Look at Symfony for example: they had that container-injecting approach as a default for controllers and commands, they had this same interface, and they deprecated and removed it, for those reasons exactly.
Look at Symfony for example
Symfony is not an ideal of proper architecture. Look at https://symfony.com/blog/new-in-symfony-5-1-autowire-public-typed-properties Are you really sure that they removed it by the reasons above? :)
From the very same link that you posted:
This is in practice equivalent to setter injection, which has drawbacks and it should be used only in very specific scenarios.
Also, from Symfony's PRs:
As easy as this seams to be, using the container directly is not considered a good practice because it hides the dependencies of your classes, making them coupled to external configuration, thus harder to test, harder to review, etc.
Still, bottomline, this proposed interface is not something that empowers interop, so it's not that should be put forward by the PHP-FIG.
What about to have this interface? With this we can do lazy passing the container to a service like the
Psr\Log\LoggerAwareInterface