{"action":"created","issue":{"url":"https://api.github.com/repos/container-interop/service-provider/issues/18","repository_url":"https://api.github.com/repos/container-interop/service-provider","labels_url":"https://api.github.com/repos/container-interop/service-provider/issues/18/labels{/name}","comments_url":"https://api.github.com/repos/container-interop/service-provider/issues/18/comments","events_url":"https://api.github.com/repos/container-interop/service-provider/issues/18/events","html_url":"https://github.com/container-interop/service-provider/issues/18","id":143859062,"number":18,"title":"A method to enforce factory methods signature?","user":{"login":"Giuseppe-Mazzapica","id":2208282,"avatar_url":"https://avatars.githubusercontent.com/u/2208282?v=3","gravatar_id":"","url":"https://api.github.com/users/Giuseppe-Mazzapica","html_url":"https://github.com/Giuseppe-Mazzapica","followers_url":"https://api.github.com/users/Giuseppe-Mazzapica/followers","following_url":"https://api.github.com/users/Giuseppe-Mazzapica/following{/other_user}","gists_url":"https://api.github.com/users/Giuseppe-Mazzapica/gists{/gist_id}","starred_url":"https://api.github.com/users/Giuseppe-Mazzapica/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Giuseppe-Mazzapica/subscriptions","organizations_url":"https://api.github.com/users/Giuseppe-Mazzapica/orgs","repos_url":"https://api.github.com/users/Giuseppe-Mazzapica/repos","events_url":"https://api.github.com/users/Giuseppe-Mazzapica/events{/privacy}","received_events_url":"https://api.github.com/users/Giuseppe-Mazzapica/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":12,"created_at":"2016-03-27T23:02:47Z","updated_at":"2016-04-06T07:51:24Z","closed_at":null,"body":"I already exposed in #16 my opinion about the fragility I see in current implementation, because the `getServices()` method is not able to enforce... anything.\r\n\r\nIt means that the definition of the standard will entirely be in the method doc block, but the `getServices()` method itself, that is everything the interface contains, is nothing more than a wrapper around an array.\r\n\r\nI think that if we are expecting people to write factory methods in the service provider, we should enforce in the interface the factory methods signature.\r\n\r\nThis is more or less what I have in mind:\r\n\r\n ```php\r\ninterface ServiceProvider\r\n{\r\n   /**\r\n    * Return an associative array of provided services where:\r\n    * each item *key* is the service name (can be used with container `get` / `has`)\r\n    * each item *value* is the the service \"type\" (class, interface or PHP scalar type).\r\n    * An invalid item value (not string or not referring any valid type) COULD throw an Exception,\r\n    * use `null` as value to safely don't enforce any specific type\r\n    *\r\n    * @return string[]\r\n    */\r\n   public function servicesMap();\r\n\r\n  /**\r\n   * Given a service name create a service instance.\r\n   * Provided container can be used to resolve dependencies.\r\n   *\r\n   * @param string $service Any of the service names, provided as array keys by servicesMap()\r\n   * @param ContainerInterface $c\r\n   * @param callable $previous If provided, can be called to obtain previous service value\r\n   * @return mixed\r\n   */\r\n   public function createService($service, ContainerInterface $c, callable $previous = null);\r\n}\r\n ```\r\n\r\nPlease note how I not used static methods. Refers to #5 for more info on the reason why. \r\n\r\nI think that, even if not ideal, this is a probably acceptable trade off and reduce the \"fragility\" of current implementation.\r\n\r\nAnother variant could be that `servicesMap`, could just return a list of services names, (no information on service types). This would simplify the interface. In that case `servicesMap` should be renamed.\r\n\r\nImplementation example:\r\n\r\n ```php\r\nnamespace MyLib;\r\n\r\nuse Interop\\Container\\ContainerInterface;\r\nuse Interop\\Container\\ServiceProvider;\r\nuse Psr\\Log\\LoggerInterface;\r\nuse Monolog\\Logger;\r\n\r\nclass LoggerServiceProvider implements ServiceProvider\r\n{\r\n\r\n     const MAP = ['logger' => LoggerInterface::class];\r\n\r\n     public function servicesMap()\r\n     {\r\n         return static::MAP;\r\n     }\r\n\r\n     public function createService($service, ContainerInterface $c, callable $previous = null)\r\n     {\r\n         if (! array_key_exists($service, static::MAP) ) {\r\n             throw new \\Exception(sprintf(\"Service %s not provided by %s.\", $service, __CLASS__));\r\n         }\r\n         $cb = [$this, $service];\r\n         $instance = $cb($c); \r\n         $this->checkService($instance);\r\n\r\n         return $instance;\r\n      }\r\n\r\n      private function logger(ContainerInterface $c)\r\n      {\r\n          return new Logger('name');\r\n      }\r\n\r\n      private function checkService($service, $instance)\r\n      {\r\n         $scalars = ['bool', 'int', 'float', 'string', 'array', 'resource'];\r\n         $checker = in_array(static::MAP[$service], $scalars, true)\r\n             ? 'is_'.static::MAP[$service]\r\n             : function($instance) use($service) { return is_a($instance, static::MAP[$service]); }\r\n       \r\n         if (!is_null(static::MAP[$service) && !$checker($instance)) {\r\n             throw new \\Exception(\r\n                sprintf(\r\n                    \"Service %s expected to be of type %s, but is of type %s instead.\",\r\n                    $service,\r\n                    static::MAP[$service],\r\n                    is_object($instance) ? get_class($instance) : gettype($instance)\r\n                )\r\n             );\r\n         }\r\n     }\r\n\r\n}\r\n ```\r\n\r\nNote how in a real world library containing more service providers, `servicesMap`,`createService` and `checkService` could be implemented in a trait or in an abstract superclass, so that each provider class could just define own `MAP` constant and the needed private factory methods.\r\n\r\nIt means the code necessary would be very similar on current implementation (RAD not affected) but this implementation is capable to enforce factory method signature and returned type.\r\n\r\nAmong other things, this has the huge benefit that `$container->get('service')` is enforced to return a predictable type (and an IDE supporting the standard could take advantage of it).\r\n\r\nFinally, if this implementation will be take into consideration, we should probably also take into consideration to provide two exception classes, one to be thrown when `createService` receives a non provided service, and one to be thrown when the service type does not match the expected type. As alternative, one exception class with 2 different status codes (provided as class constants) should do.\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/container-interop/service-provider/issues/comments/206190994","html_url":"https://github.com/container-interop/service-provider/issues/18#issuecomment-206190994","issue_url":"https://api.github.com/repos/container-interop/service-provider/issues/18","id":206190994,"user":{"login":"mnapoli","id":720328,"avatar_url":"https://avatars.githubusercontent.com/u/720328?v=3","gravatar_id":"","url":"https://api.github.com/users/mnapoli","html_url":"https://github.com/mnapoli","followers_url":"https://api.github.com/users/mnapoli/followers","following_url":"https://api.github.com/users/mnapoli/following{/other_user}","gists_url":"https://api.github.com/users/mnapoli/gists{/gist_id}","starred_url":"https://api.github.com/users/mnapoli/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mnapoli/subscriptions","organizations_url":"https://api.github.com/users/mnapoli/orgs","repos_url":"https://api.github.com/users/mnapoli/repos","events_url":"https://api.github.com/users/mnapoli/events{/privacy}","received_events_url":"https://api.github.com/users/mnapoli/received_events","type":"User","site_admin":false},"created_at":"2016-04-06T07:51:24Z","updated_at":"2016-04-06T07:51:24Z","body":"> First of all there's no \"my 1st solution\" and \"my 2nd\" solution: those \"1st\" and \"2nd\" are implementation examples.\r\n\r\nWe need to consider how interfaces will be implemented. It doesn't make sense to consider the interface alone ore we'll only be talking about smoke. If it's not possible to write an implementation that is clear and efficient then the interface needs to be improved.\r\n\r\n> Let's admit that put all the service factories inside one method is a big flaw: this is a implementation detail, not something that is enforced by the interface\r\n\r\nExactly:\r\n\r\n- it's a design flaw\r\n- it can't be enforced by your interface suggestion\r\n\r\nOn the other hand the current solution doesn't have this problem because it can enforce it.\r\n\r\n> 1 call to getServices() which returns 3 methods names\r\n\r\nWe don't need to consider this call because it will most probably not be done at runtime… (for obvious reasons)\r\n\r\n> Which may became 2 VS 11 for a service provider that provides 10 services.\r\n\r\nI don't understand. `createService()` needs to be called 10 times, because one call to this method will only return 1 service, not 10. So it's the same amount of calls at minimum.\r\n\r\n> The point is that the real number of methods depends on implementation. And since you can't know how people will implement the interface\r\n\r\nMaybe with your solution we can't know but with the current one we know.\r\n\r\n> Considering that the final target of the standard of is an interface, we should only talk about which is the best interface not the best implementation.\r\n\r\nThis is wrong on many levels. Caring about a standards but not about implementations is the best way to fail. We have to take real implementation constraints into account.\r\n\r\nHave you tried implementing your suggestion into your DI container or your framework (or any existing container or framework)? The PHP-FIG members don't want to waste time on theoretical ideas, that's why we spend time actually implementing and testing what we design. We spend considerable amount of time writing adapters and implementations to have real experience on what we suggest. Drafting something on a paper on a GitHub issue is 5% of the job. That's why we are discussing details."}}