More often one would like to have an address, returned by reverse geocoding service, as a structure, with separate fields for all the parts an address consists of. Returning a formatted address doesn't cut it most of the times. Also (as a personal opinion of mine and engineers in my team) I find it nice having homogeneous definitions - Geocode returns the Location structure, so why not have a similar signature for ReverseGeocode. Hence I would like to propose (and implement it all over) the following change in the Geocoder interface definitions:

type Geocoder interface {
	Geocode(address string) (*Location, error)
	ReverseGeocode(lat, lng float64) (*Address, error)
}

with Address being defined in the package geo in a similar manner to the Location one

// Address is the structured representation of an address, including its flat representation
type Address struct {
	FormattedAddress string
	Street           string
	HouseNumber      string
	Suburb           string
	Postcode         string
	State            string
	StateDistrict    string
	County           string
	Country          string
	CountryCode      string
	City             string
}

My reasoning for the further proposed changes:

• Return pointer to the defined structure, so that the caller can check for nil; checking for zero values to deduce ErrNoResult may often go wrong (further no result being returned shouldn't be an error)
• Add second return value of type error makes is easier to catch errors and propagate them back to the caller, next to a nil, who then should decide how to react
• The (uniform) address string, currently returned by Address() would be accessible through the FormattedAddress field

Testing would be easier to perform and more reliable if the library used mock http responses for geo APIs (e.g. the API's seem to be a little flakey sometimes causing intermittent build failures).

We could record HTTP requests/responses from the existing tests as the initial mock http responses.

We could always conditionally run the tests in "integration" mode which would run the mock tests and actual API tests if we still wanted to exercise the live APIs.

PR to solve issue #29

It's been a while, but with Christmas, work and family added to the fact I had to sign up for many of the providers, the library supports, look into the documentation of their API, etc...

To sum up the major changes I made:

• use context.Context with a timeout instead of time.After with the benefit of cleaning up / releasing resources in case the deadline has been reached
• moved the structures geo.Location and geo.Address to geocoder.go, where the definition of the interface is placed, since these are part of the interface's contract and also to reduce the LOC in http_geocoder.go
• no longer using ErrNoResults, this can be now deduced if the returned values are nil, nil; in case an error has been returned by the provider (only few really do so) it's then translated in the second return value
• moved the examples code from geocoder_test.go to examples/geocoder_example.go to not be mistaken with unit tests, because of language specific conventions

In case of further questions / concerns, I'll be glad to discuss them.

Cheers

I'm getting some odd results like getting a nil address or location when geocoding or reverse geocoding.

Geocoding "4000 Blackhawk Plaza Cir, Danville, California 94506, United States": [Here] Geocoded '4000 Blackhawk Plaza Cir, Danville, California 94506, United States' to location: &{37.7999631 -121.9202164} [OpenCage] Failed to geocode address: geocoding error: missing API key [OpenStreetMap] Failed to geocode address: location is nil [Bing] Geocoded '4000 Blackhawk Plaza Cir, Danville, California 94506, United States' to location: &{37.800872802734375 -121.9173355102539} [Google] Geocoded '4000 Blackhawk Plaza Cir, Danville, California 94506, United States' to location: &{37.8006389 -121.919588} [Nominatim] Geocoded '4000 Blackhawk Plaza Cir, Danville, California 94506, United States' to location: &{0 0} [Open] Geocoded '4000 Blackhawk Plaza Cir, Danville, California 94506, United States' to location: &{39.78373 -100.445882} [MapBox] Geocoded '4000 Blackhawk Plaza Cir, Danville, California 94506, United States' to location: &{37.800639 -121.919588} [LocationIQ] Failed to geocode address: location is nil

Reverse Geocoding "37.8006389,-121.919588": [Google] Failed to reverse geocode location: address is nil [Here] Reverse geocoded location 37.8006389,-121.919588 to 'Danville, CA 94506, United States' [Open] Reverse geocoded location 37.8006389,-121.919588 to 'Blackhawk Plaza Circle, 94506, Blackhawk, CA, US' [MapBox] Reverse geocoded location 37.8006389,-121.919588 to '4000 Blackhawk Plaza Cir, Danville, California 94506, United States' [OpenStreetMap] Reverse geocoded location 37.8006389,-121.919588 to '4014, Blackhawk Plaza Circle, Blackhawk, Contra Costa County, California, 94506, United States of America' [Bing] Reverse geocoded location 37.8006389,-121.919588 to 'Street, Danville, CA 94506' [OpenCage] Failed to reverse geocode location: geocoding error: missing API key [LocationIQ] Reverse geocoded location 37.8006389,-121.919588 to '4014, Blackhawk Plaza Circle, Blackhawk, Contra Costa County, California, 94506, United States of America' [Nominatim] Reverse geocoded location 37.8006389,-121.919588 to 'Blackhawk Plaza Circle, Blackhawk, Contra Costa County, California, 94506, United States of America'

I'm not sure if I'm doing something wrong or my expectation is incorrect but I would imagine we should get an error if an address or location is nil.

I have a test that uses lat/long 0,0 and it seems to panic the library.

panic: runtime error: index out of range

goroutine 76 [running]: panic(0x39cd00, 0xc4200140f0) /usr/local/Cellar/go/1.7.1/libexec/src/runtime/panic.go:500 +0x1a1 ../vendor/github.com/codingsince1985/geo-golang/mapquest/open.(*geocodeResponse).Address(0xc42043c5c0, 0x50, 0x5cd2e0) ../vendor/github.com/codingsince1985/geo-golang/mapquest/open/geocoder.go:49 +0x18a ../vendor/github.com/codingsince1985/geo-golang.HTTPGeocoder.ReverseGeocode.func1(0x5ce920, 0xc4200156f0, 0x444178, 0x0, 0x0, 0xc42013b3e0) ../vendor/github.com/codingsince1985/geo-golang/http_geocoder.go:71 +0xaa created by ../vendor/github.com/codingsince1985/geo-golang.HTTPGeocoder.ReverseGeocode ../vendor/github.com/codingsince1985/geo-golang/http_geocoder.go:72 +0xb7 exit status 2

Depends on #17

• Implements a CachedGeocoder that will cache responses to another Geocoder
  • currently uses in-memory cache https://github.com/patrickmn/go-cache
    • could use a cache interface type later to support other cache implementations

Closes https://github.com/codingsince1985/geo-golang/issues/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 https://github.com/codingsince1985/geo-golang/issues/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
}

Seeing another panic, not sure if it is tracked or not.

I think it's this line: func (r *geocodeResponse) Location() geo.Location { return r.Results[0].Locations[0].LatLng }

panic: runtime error: index out of range

goroutine 15 [running]: panic(0x3a0bc0, 0xc420016100) /usr/local/Cellar/go/1.7.1/libexec/src/runtime/panic.go:500 +0x1a1 ../codingsince1985/geo-golang/mapquest/open.(*geocodeResponse).Location(0xc4204323a0, 0x6d, 0x5d32e0) /Users/saheljalal/code/go/src/../codingsince1985/geo-golang/mapquest/open/geocoder.go:46 +0x47 ../codingsince1985/geo-golang.HTTPGeocoder.Geocode.func1(0x5d4920, 0xc420017700, 0x448aa8, 0xc42014cbd0, 0x2e, 0xc420302d90) /Users/saheljalal/code/go/src/../codingsince1985/geo-golang/http_geocoder.go:54 +0xbe created by ../codingsince1985/geo-golang.HTTPGeocoder.Geocode /Users/saheljalal/code/go/src/../codingsince1985/geo-golang/http_geocoder.go:55 +0xb6

Hi

I want to use your library, but need State Code in the response. Please, check my solution, I've added state code in all services which provide state code.

I can make additional updates if something wrong.

Otherwise, I'll need to create a map where I'll lookup long names to short, which looks strange for me as logically library can provide these already.

Regards, Aleksandr

we are using opencage as provider and some times when we do a request for it we receive this error invalid character '<' looking for beginning of value and would be nice if we could see the response from opencage. So with this PR we create a Logger for those who use this library print the payload and it is flexible.

For instance for address: 31171 Fisher Manor,Simeonbury,United States

panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1228a40]

Because func response from http_geocoder.go returns nil when body is empty.

Would you accpet a PR, or consider modifying this module such that users could pass a context to the Geocoder interface? This addition would make this module much more powerful in production, since web servers would be able to pass the top level context for things like tracing and etc. The new interface would not be backwards compatible, but would be trivial for users to update, i.e. provide context.TODO to their calls as you've done

Sometimes we need to specify some more detailed query params. For example here we have a lack of language and region parameters for google geocoding api

I only want to use the lib only to autocomplete adresses. I want to search for "Fred-Grube-Platz 23, 30451 Hannover" and get the "formattedAdress" back. IS that possible? Or is only a conversion from adress to lat/lon possible?

Hello,

By default the package does not include headers related to language, ie Accept-Language

Using, Accept-Language = en for example will result in the full name of the city/state

While, Accept-Language = EN for example will result in abbreviation for the city/state

In my setup I often see the later being returned which results in abbreviations for city/state.

Can we introduce an optional param to Geocoder to pass in headers when making a request to Nominatim (so that we can control the Accept-Language header)?

Thank you!

More about this issue is outlined here: https://github.com/openstreetmap/Nominatim/issues/783

Some providers do give certain details about what type of result it is. Having this information (when available) will help validate the results, and can help determine if I need to use another provider as a fallback if the results is not accurate enough.