search

expectedbehavior / php-docraptor

PHP consumer for the DocRaptor.com API
ISC License
7 stars 1 forks source link

Move API level options to Config class. #34

Closed janxious closed 9 years ago

janxious commented 9 years ago

For example: https vs http.

markushausammann commented 9 years ago

Not sure that's a good idea, for example https vs http really are property of the wrapper. Maybe I want to have two docraptor instances, one with http and one with https, etc. I think most or all the properties we have in the wrapper belong there... I'm already skeptical about the special config class as is.

janxious commented 9 years ago

Yeah, I agree with you somewhat on that being a property of the Wrapper. I think maybe there should be zero or two configs here. One for wrapper, one fore http client. I guess there could remain one but use the ApiWrapper to factory up a specific httpclient we want per request.

How would you like config to work?

markushausammann commented 9 years ago

It's fine as it is. At the moment the Config class contains only meta config, that's ok. I just wouldn't extract default values from the other classes into a config. IMO it's totally fine to have that config in the context it belongs. No need for factories either I think.