-
Notifications
You must be signed in to change notification settings - Fork 151
Feat/support dependency injection #115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…net into namespaces-reorg
@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. |
Closes #95 |
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. |
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 ***@***.***> 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
<#115 (comment)>,
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]
|
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: 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. |
…wton76/gmaps-api-net into feat/support-dependency-injection
Implemented constructor based dependency injection
Created new type Google.Maps.Services that perform default (poor-mans-DI-framework) injection for ease of use.