Expose Executer via factory class

[skydiablo]

this allows to extend the give classes and more flexible for own use.

[clue]

@skydiablo Thank you very much for looking into this and filing this PR!

I can definitely see some room for improvement for the ResolverInterface, but my biggest concern here is that adding a new method to an interfaces is inherently a BC break, as it will break all existing implementors of this interface.

We've had a related discussion in #159 (and others), so I wonder how much is really gained by having this as part of the ResolverInterface instead of using the existing ExecutorInterface?

(I've added minor remarks below for completeness, but my main concern at the moment would be the BC break mentioned above.)

[skydiablo]

my motivation is to benefit from the existing Resolver-Factory and also have access to the underlying executor, in detail to the query function. in the current implementation there is no way to get this in one shut, i have to copy-paste the complete factory class and inject my requirements.
with this changes, i can just extend the factory class and override the points to met my requirements.

[clue]

my motivation is to benefit from the existing Resolver-Factory and also have access to the underlying executor, in detail to the query function. in the current implementation there is no way to get this in one shut […]

@skydiablo Very true, completely agree that this is needlessly complicated. Perhaps we can simplify this by simply exposing the ExecutorInterface from the Factory somehow? The ExecutorInterface can still be passed to the Resolver if you need to work with the ResolverInterfaec. This way, you should be able to invoke all methods using the same ExecutorInterface. WDYT?

[skydiablo]

@clue just expose an function like createExecuter for internal and external use in the Factory-Class? Or What about an new Factory-Class still for the Executer itself and Resolver-Factory can use this Factory internal itself or just extends the Executer-Factory?

[clue]

just expose an function like createExecuter for internal and external use in the Factory-Class?

Sounds about right, but just an idea at this point and we'd have to take a look at this more in-depth.

Or What about an new Factory-Class still for the Executer itself and Resolver-Factory can use this Factory internal itself or just extends the Executer-Factory?

Extending implies an "is a" relationship, not sure "ResolverFactory is a ExecutorFactory" is reasonable. Composition over inheritance?

Not sure a dedicated ExecutorFactory is worth the effort, especially since we've started moving away from factory classes where possible to improve DX. I agree that adding this as a method to the ResolverFactory might not be perfect, but perhaps might make sense from a KISS perspective?

[skydiablo]

divide the two section should the best case and the last change does not have any BC breaks. in case of DI, nobody say you have to use a factory-class, it will just create for you the "best" case of composition.

[WyriHaximus]

This pretty much creates what most end users need to use this component. IMHO using Factory as name tells enough, and since this object holds no state, we can make all methods static.

[WyriHaximus]

Never mind this comment's part on naming, missed that it is in the Query directory. But we can still make all methods static IMHO.

[skydiablo]

hm, yes it is possible.... @clue ???

[clue]

No strong opinion here, but not sure I quite like the idea of going more into factory land. I understand it's not optimal, but perhaps adding a new method to the existing factory class helps reduce the API surface?

[skydiablo]

in respect to the upcoming DI/DX in react, a more granular distribution of classes give the best opportunity to combine it in more fancy ways and a test-case can be more precisely. so we should it keep in two classes. the static way can be interest but not really needed, i would it leave as it is ...

[WyriHaximus]

Been thinking about this over the weekend, and part of the issue of creating another factory is that we'd start putting them everywhere. And you have to start creating aliases for them if you use more than one. Not hard opinions here either.

[skydiablo]

as long as the reactPHP libs does not depend and implement an DI/DX container, a factory is a good alternate. later we can refactor to DI and can exchange or run in parallel mode.

[skydiablo]

push :P