alexrabarts / iso_country_codes

A Ruby library that provides ISO 3166-1 country codes/names and ISO 4217 currencies
http://github.com/alexrabarts/iso-country-codes
163 stars 62 forks source link

Option to skip UnknownCodeError when code not found #11

Closed benjaminwood closed 10 years ago

benjaminwood commented 10 years ago

@alexrabarts thanks for your work on this. I'm using it in a production app!

Are you still maintaining this project? I'd be happy to step up to the plate and implement this myself if you're still around and merging pool requests.

I'm using the gem in a save callback on a model in conjunction with a geocoding service that is returning country codes. I convert the country code to the country name using iso_country_codes. It makes me nervous that the third party service could return a new or invalid country code and prevent the model from saving. So, I'm doing something like this:

user.country = begin; IsoCountryCodes.find(geo.country).name; rescue; geo.country; end;

It'd be handy if there was a configuration option to return whatever was passed to the find method if no country is found instead of raising an error.

vjt commented 10 years ago

Hi! :smiley:

I am not the gem author nor a contributor - just maintaining a :fork_and_knife:.

I don't think what you are proposing is a good idea. The gem API should be solid and well-determined, and :boom: when something is wrong.

What you are asking is to make the gem handle your app's concern: that is scope creep :smiley:.

I suggest you to refactor your code as follows, assuming your User is an ActiveRecord::Base:

class User < ActiveRecord::Base

  def country=(code)
    write_attribute(:country, country_name_from(code))
  end

  private
    def country_name_from(code)
      IsoCountryCodes.find(code)
    rescue IsoCountryCode::UnknownCodeError
      code
    end

This way you liberate your controller from dealing with these errors, and you isolate country name fetching in a separate method, easy to refactor :neckbeard:

benjaminwood commented 10 years ago

Hey @vjt, yeah that makes some good sense! :-)

In general I agree with you. I guess it comes down to what you consider 'something wrong'. I guess what I originally expected was to see it return nil if the country wasn't found. I see there are problems with that too, though.

Anyway, I suppose you're right. It's not hard to handle this properly on my end. :-)

Thanks for your time and your reply. Cheers!

vjt commented 10 years ago

:beers: :smile: