search

Nyholm / psr7-server

Helper classes to use any PSR7 implementation as your main request and response
MIT License
91 stars 21 forks source link

Stop using UriFactory #18

Closed Nyholm closed 4 years ago

Nyholm commented 6 years ago

If we rewrite this function so it does not use the UriFactory but returning a URI string instead. Then we could pass that string directly to the ServerRequestFactory.

Is that a refactoring we want to do?

Zegnat commented 6 years ago

I am not sure what the pros/cons are of this. The only thing I can think of is that ServerRequestCreator gets to have 1 less factory to depend on?

Nyholm commented 6 years ago

Exactly. That is the benefit of it. We basically hide that dependency to the ServerRequestFactory implementation. Or, maybe not hide it because the ServerRequestFactory implementation will just do new Uri($uri)

Zegnat commented 6 years ago

I don’t really care about hiding one dependency. Anyone with a good DI setup will have the dependencies automatically passed to the constructor anyway.

Are we (theoretically) providing better performance the way we do it now? Because we already have access to a parsed URL (in the loosest form), and if we concatenate it into a single long string the underlying Uri / ServerRequestFactory will have to re-parse it again.

I think we should aim for whatever is the most performant or stable option, rather than the option that lets us cut an argument from __construct.