factories use container as service locator and uses classnames as keys.. #23

Activity

1. snapshotpl commented on Oct 10, 2018

   

   snapshotpl

   on Oct 10, 2018

   Member

   More actions

   @ppetermann thanks your feedback

   1. As injecting container into your classes is anti-pattern - that's usage in this package is absolutely right. Provided factories exists on "container" level - can be use to build object using object, following container implementation. See how zend-servicemanager, pimple create services - by invokable objects. There isn't any other object where container is injected.
   2. Can you provide example?
   3. config key - you have right. It can be done better. However what is problem with final class and loading config from file? Provided config is just default set of data - you should get it and modify by your needs.
2. ppetermann commented on Oct 14, 2018

   

   ppetermann

   on Oct 14, 2018

   Author

   More actions

   1) injection of the container into the factories

   I just realized from what you wrote, and from looking at how you suggest using this with slim, that you don't intent to use the factory as a registered service, but that the factory is the registration of the service it provides (meaning, it replaces the function () {return new Service();}), and that is why you inject the container where you do (as that is how pimple can deal with it)
   So you are right, the use is OK, I was mislead by just reading the Installation part, and skipping the parts about frameworks that I currently don't use.

   2) example for interface rather than classname

   Sure

   • components should depend on the public interface of the service they use, not on the implementation
   • the goal of IoC, which DI is a solution for is to decouple the implementation from the use of a component, so you end up with only depending on the interface
   • decoupling is done so it is easy to replace parts

   say you use a key like JavascriptRenderer::class:

   <?php
   // set it
   $container[JavascriptRenderer::class] = function($c) { return new JavascriptRenderer();}
   
   // so when you use it somewhere you'd do:
   $container[PhpDebugBarMiddleware::class] = function ($c) {
     return new PhpDebugBarMiddleware($container->get(JavascriptRenderer::class));
   }
   
   class PhpDebugBarMiddleware 
   {
     public function __construct(JavacscriptRenderer $debugbarRenderer)
     {
     }
   }

   So have you really decoupled from Javascript renderer here? nope, because
   your Middleware is still expecting an implementation of JavascriptRenderer, so replacing it would mean to load a class of the exact same name, and prevent the one of PhpDebugBar from loading.

   now if PhpDebugBar was a well designed Software, it would provide a RendererInterface, which sadly, it is not, so for solving the dependency we should clean this up by

   <?php
   interface RendererInterface 
   {
     public function render(); // i know there is more, but lets keep the example short
   }
   
   
   class WrappedJavascriptRenderer implements RendererInterface
   {
     private $delegate;
     public function __construct()
     {
       // we could inject this too, but since this is the replaceable component, no point in that
       $this->delegate = new JavascriptRenderer();
     }
   
     public function render()
     {
       return $this->delegate->render();
     }
   }

   Now instead of depending on JavascriptRenderer, the middleware can depend on a replaceable component whose interface is known

   <?php
   class PhpDebugBarMiddleware 
   {
     public function __construct(RendererInterface $debugbarRenderer)
     {
       
     }
   }

   so from this moment on the PhpDebugMiddleware would be actually decoupled,
   now any class implementing the interface could be used to replace JavascriptRenderer

   but wait! there is more! its still using the classname for lookups, which means
   either you have to register a replacing component under the classname of another component,
   or you replace the key by the interface name, which means you will consistently use the interface
   name everywhere, and have only one place where the actual classname of the implementation is used:
   the place where the Implementing component is registered to the dependency injection

   <?php
   $container[RendererInterface::class] = function($c) { return new WrappedJavascriptRenderer();}
   
   $container[PhpDebugBarMiddleware::class] = function ($c) {
     return new PhpDebugBarMiddleware($container->get(RendererInterface::class));
   }

   ... which leaves you with a lot cleaner setup, where things are easy to replace, while keeping a consistent and correct name.
   Bonus Points: if you do that, there are containers out there which can autowire typehint to key in container, thus making it a bit less boilerplate for those who work with them, while still retaining all benefits.

   I've left out namespaces, and i might have typos in the code above. however if i forgot anything else, don't hesitate to ask

   3) config "service", and ConfigProvider

   Yeah, i think for ConfigProvider i fell into the same trap as in 1), I didn't notice it was Zend specific.
   However having the ambigous "config" key, witht he requirement of it being an array (well i guess ArrayAccess would do) might conflict with what some frameworks do, and isn't a very clean interface either (I still hope the FIG folks get of their asses, and start putting a few config interfaces up, but as it stands every framework does their own thing here..))