Skip to content
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

Expose http.Client #28

Closed
enj opened this issue Jan 4, 2015 · 3 comments
Closed

Expose http.Client #28

enj opened this issue Jan 4, 2015 · 3 comments

Comments

@enj
Copy link

enj commented Jan 4, 2015

I am trying to use this library with Google App Engine which does not allow one to use the standard library to make HTTP requests. I ran into a similar issue with a Twitter API, but was able to replace the http.Client since it was exposed in the API.

I believe a change like this could allow for a similar capability:

// This struct contains all the funcitonality
// of interacting with the Google Maps Geocoding Service
type GoogleGeocoder struct{
    HttpClient    *http.Client
}

...

// Issues a request to the google geocoding service and forwards the passed in params string
// as a URL-encoded entity.  Returns an array of byes as a result, or an error if one occurs during the process.
func (g *GoogleGeocoder) Request(params string) ([]byte, error) {
    if g.HttpClient == nil {
        g.HttpClient = &http.Client{}
    }
    client := g.HttpClient
    ...

Thoughts?

@kellydunn
Copy link
Owner

I think this could be a valuable addition, especially since you could swap out the http client to a mock client for testing purposes.

My only piece of feedback is to "constant-ize" the newly instantiated &http.Client{} as DefaultHTTPClient, like so:

var DefaultHTTPClient = &http.Client{}

//...

func (g *GoogleGeocoder) Request(params string) ([]byte, error) {
  if g.HttpClient == nil {
    g.HttpClient = DefaultHTTPClient
  }
  client := g.HttpClient
}

Fwiw, I have a list of features that I'd like to see in a 1.0.0 release of golang-geo, and this one could be on that list, as I see it being valuable for all geocoders to have this ability.

@kellydunn
Copy link
Owner

Great, It's on master now.

@enj , want to try it out and let me know if there's anything missing? If all is well, I'll close out this ticket 😄 🍻

@enj
Copy link
Author

enj commented Jan 14, 2015

Works like a charm, feel free to close. Thanks for the help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants