search

jgv / area

Perform a variety of conversions between places and area codes or zip codes.
http://jonathanvingiano.com/area
MIT License
176 stars 58 forks source link

fix converting string zipcode to gmt offset #5

Closed aaronfeng closed 11 years ago

aaronfeng commented 11 years ago

Previously it was not considering zipcode column from the csv file during the to gmt conversion.

jgv commented 11 years ago

Thanks Aaron! :beers: :computer: :beers:

aaronfeng commented 11 years ago

np, glad I can help. FYI, there's a lot of room for perf improvements. Every operation is iterating through all the possible values even when it doesn't have to. An easy fix would be to break out of the loop when the value is found.

Also, some of the operations might be questionable on integers. For example, "08034".to_i.to_gmt_offset doesn't really work as well as "90210".to_i.to_gmt_offset.

jgv commented 11 years ago

Thanks for the tip on improving performance. Zipcodes with leading zeros are problematic. Ruby will convert "08034".to_i to 8034 because of the leading zeros. So when converting from a zip to GMT offset, the zip will have to be a string (https://github.com/jgv/area#get-the-gmt-offset-of-a-zipcode). Maybe there is a way around this with sprintf or rjust.

aaronfeng commented 11 years ago

Yes, I understand the reason. One way to get around the int zipcode issue is to convert to string then pad the left side with zeros. Or just not support it by throwing an exception :)

Another note on perf, ideally, the lookup will happen via hash not array like data structure. If would be nice if the lookup is O(1) instead of O(n). In some cases, I see O(n^2) complexity in area. Not sure how much work will this require, but it's worth thinking about.