Feat/support dependency injection

[ericnewton76]

Implemented constructor based dependency injection

Created new type Google.Maps.Services that perform default (poor-mans-DI-framework) injection for ease of use.

[ericnewton76]

@richardthombs was looking for a review and comments of this.

This is a change to the public API to better support dependency injection and to fix a couple of issues related to testing and mocking (ie by removing static GoogleSigned property) and the like.

[ericnewton76]

Closes #95

[richardthombs]

I like the idea of abstracting the repeated GetResponse and GetResponseAsync into an abstract base class, but I think the the interfaces that presumably you intend to use for DI are really complicated and too low level.

If I wanted to mock say the GeocodingService, I'd far rather interact with an IGeocodingService interface than an IWhatever<IGeocodingRequest,IGeocodingResponse>.

I think trying to put an interface on the request and response objects is going too far. I've not seen anything else do that and I struggle to think of any use case where that would be useful.

[ericnewton76]

That part wasnt really the DI part, that was just establishing a base class for all the services that unified the interfaces a bit. Although it started to break down when the underlying service had multiple action responses (like StaticMapService and PlacesService and a couple other ones)

The DI part was that each service takes its dependencies via its constructors, ie an instance of an HttpHandler and an instance of a GoogleSigned. The HttpHandler will allow for the Exponential Backoff http strategy for dealing with overlimit responses.

And the GMaps type was created to make it easy to use if you didnt care about DI. It basically acts as the DI container and a services factory, creating the dependencies for the services and giving you the instance.

On Sat, Nov 11, 2017 at 2:36 AM, Richard Thombs [email protected] wrote:

I like the idea of abstracting the repeated GetResponse and GetResponseAsync into an abstract base class, but I think the the interfaces that presumably you intend to use for DI are really complicated and too low level.

If I wanted to mock say the GeocodingService, I'd far rather interact with an IGeocodingService interface than an IWhatever<IGeocodingRequest, IGeocodingResponse>.

I think trying to put an interface on the request and response objects is going too far. I've not seen anything else do that and I struggle to think of any use case where that would be useful.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ericnewton76/gmaps-api-net/pull/115#issuecomment-343647127, or mute the thread https://github.com/notifications/unsubscribe-auth/ACG_zDQAJ8CtgVnXZKPym3OVaC7kA-7pks5s1U6NgaJpZM4QKm29 .

--

E.Newton Senior Software Craftsman Cell: 407-497-1429 EMail: [email protected]

[richardthombs]

I don't know... this breaks API compatibility for no obvious reason.

I think that backoff could be added directly into MapsHttp and then enabled by setting a service property. Eg: new GeocodingService { Retries = 10 } or something similar. If the property defaulted to 1, then both the API and the default behaviour would be entirely backward compatible.

We can argue back and forth about whether it would be better to refactor the code internally so that it uses DI, but if it involves breaking the API then I think it is better pushed to version 2.0.