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

Geocoder as an interface to allow for chained and cached types #16

Merged
merged 1 commit into from
Apr 15, 2016

Conversation

dougnukem
Copy link
Collaborator

Closes #10

Make the top-level Geocoder type an interface and the HTTP specific one a HTTPGeocoder

This will be useful for implementing more abstract Geocoder types like Chained fallback ones or cached ones as described here #10

Also made ResponseObject() be a function that returns new instances (I think there may have been a concurrency issue because the Geocoders were returning the same pointer for every call, so if you made concurrent calls to a Geocoder API it would be decoding to the same response pointer).

// Geocoder can look up (lat, long) by address and address by (lat, long)
type Geocoder interface {
    Geocode(address string) (Location, error)
    ReverseGeocode(lat, lng float64) (string, error)
}
// HTTPGeocoder has EndpointBuilder and ResponseParser
type HTTPGeocoder struct {
    EndpointBuilder
    ResponseParserFactory ResponseParserFactory
}

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 96.476% when pulling d7c5d30 on doug/geocoder_interface into b0ba6eb on master.

@dougnukem
Copy link
Collaborator Author

ptal @codingsince1985 this is a fairly opinionated refactor to turn Geocoder into an interface, but it should clean up things and allow for new possibilities like composing Geocoders to fallback or cached Geocoders.

@dougnukem
Copy link
Collaborator Author

@codingsince1985 we might want to change the coveralls.io "failure" threshold to something like 5% or 10% drop in code coverage instead of reporting any drop as a failure (e.g. when we remove code such that there's less total lines to be covered)

@dougnukem
Copy link
Collaborator Author

@codingsince1985 I rebased my PR's so if you think they're good to go we can merge them in

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 96.476% when pulling 6ce8551 on doug/geocoder_interface into e09cd84 on master.

@codingsince1985 codingsince1985 merged commit 2910405 into master Apr 15, 2016
@codingsince1985 codingsince1985 deleted the doug/geocoder_interface branch April 15, 2016 09:41
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

Successfully merging this pull request may close these issues.

3 participants