Do not use discovery in client

[sagikazarmark]

Currently we have clients like this:

class Client implements HttpClient
{
    public function __construct(MessageFactory $mf = null)
    {
       // Use discovery if MF is not passed
    }
}

I think we should rather force the user to pass a message factory and use the discovery manually OR use a static constructor, like Client::zeroConfig to setup the client.

The big advantage would be forcing DI and removing discovery dependency from client implementations.

[dbu]

in a context like the symfony dependency injection container, its very convenient to have the same constructor and just pass null if we have no explicit value. with separate methods, the bundle would either need some sort of factory or a lot more code to rewrite the container based on whether we have the factory explicitly or not.

we already encourage DI, and having a ::zeroConfig method would still keep dependency on the discovery.

then again, if puli can conveniently provide that factory in a symfony DI context, we could drop the null option and not even offer ::zeroConfig but instead document utils and the puli thing from #28.

[sagikazarmark]

Puli can provide its discovery class as a service:

https://github.com/puli/symfony-bundle/blob/master/Resources/config/services-2.7.xml#L12

However, it cannot return a "most fitting" client, it can only return all of the available ones. Maybe we could wrap our service around it which finds the most fitting one.

[dbu]

what does HttplugDiscovery::find('Http\Client\HttpClient') that you mention in #28 return?

@webmozart can the puli symfony bundle provide a service to be used in the DI?

[sagikazarmark]

It would return one instance, as in case of our current discovery classes. But Puli discovery returns all the fitting implementations.

Indeed it would be cool if we could just use Puli to provide services for class binding types in symfony. Although in this case our custom logic in case of message factories (only return a message factory if its dependent class exists) would not work. We would have to create bridge packages for Guzzle PSR7 and Diactoros which actually require them.

[webmozart]

I think you can wrap a very simple class around Puli's Discovery that implements your custom selection logic on the bindings returned by Puli. I don't really want to add that to Puli, since what a binding is and how it is used depends a lot on the use case.

[sagikazarmark]

Yeah, that's the plan. My idea regarding puli was rather something like findOneByType. Useful when you don't care about the implementation, just want to use something that is available for that type.

[webmozart]

The problem in this case is: How do you decide which one to use if there is more than one? You'd need to support some sort of selection logic.

[sagikazarmark]

One possibility is to return the first one. Fits exactly the case I specified.

Another possibility is what you already have in the find method: expressions. This still does not allow you to sort the order of resources, but you can limit them to your needs.

If you need more advanced logic than the above, then you can still use the original find method.

[Nyholm]

It has been a great while since this issue was opened. The discovery layer is way lighter then 6 months ago. I do not see a problem of using Discovery in the client constructor like we do in Guzzle5.

Can this issue be closed?

[dbu]

+1 for closing and have the clients depend on discovery for this.