sleepycat / geox

Geocoding meets GraphQL
ISC License
2 stars 0 forks source link

Drop geocodeExact #5

Open DenisCarriere opened 8 years ago

DenisCarriere commented 8 years ago

This function is a giant mess with tons of confusing catch/errors.

I'd recommend you drop this function entirely and focus on a "core" single geocode method, this function looks very specific and could be added into the users code after he receives the json results.

export async function geocodeExact(address: string, query = defaultQuery): Promise<GoogleResults> {
  const url = `https://maps.googleapis.com/maps/api/geocode/json?address=${ encodeURI(address) }&sensor=false`
  let response: any
  let json: GoogleResults

  let promise: Promise<GoogleResults> = new Promise(async (resolve, reject) => {
    try {
      response = await fetch(url)
    } catch (error) {
      reject(Error(`Couldn't connect to Geocoding API.`))
    }

    try {
      json = response.json()
    } catch (error) {
      return reject(Error(`Failed to get json.`))
    }

    let preciseResults = json.results.filter((result) => {
      if (result.geometry.location_type !== 'APPROXIMATE') {
        return result
      }
    })

    if (preciseResults.length > 0) {
      try {
        return resolve(graphql(resolver, query, {results: preciseResults}))
      } catch (error) {
        return reject(Error(error))
      }
    } else {
      return reject(Error(`All results were approximate`))
    }
  })
  return promise
}
sleepycat commented 8 years ago

I totally agree that some refactoring is needed. I was getting a lot of unhandledPromiseRejectionWarning: Unhandled promise rejection warnings and got a little carried away. :smile:

Funnily enough this function is what drove me to write this library. I guess I see it as "get me a precise location or die", which seems reasonable to expect that my geocoding library should know how to do that.

The downside (which I suspect is what you have in mind) is that I think only Google gives this information, which means this function (and by extension the library) is then tied to Google.

Thoughts?

DenisCarriere commented 8 years ago

@sleepycat yep this result is only tied to Google, every other Geocoding provider has some kind of different syntax or tag or level of measurement for accuracy, it's VERY annoying to normalize all this across every provider. That's why I created a confidence score based on a bbox since most all providers offer a bounding box.

    @property
    def confidence(self):
        if self.bbox:
            # Units are measured in Kilometers
            distance = Distance(self.northeast, self.southwest, units='km')
            for score, maximum in [(10, 0.25),
                                   (9, 0.5),
                                   (8, 1),
                                   (7, 5),
                                   (6, 7.5),
                                   (5, 10),
                                   (4, 15),
                                   (3, 20),
                                   (2, 25)]:
                if distance < maximum:
                    return score
                if distance >= 25:
                    return 1
        # Cannot determine score
        return 0

This could easily be converted into Javascript.

It gives a rating of 1 (worst) to 10 (best).

Then you're geocoder result can gage at what level of accuracy you want to use.