CODEWITHSUNDEEP
rubyarraysdistinct
Steve
Steve

Reputation: 3080

Uniq of ruby array fails to work

I have a array of my object Country which has the attributes "code" and "name"

The array could have a country in it more than once so I want to distinct the array.

This is my countries class

class Country
  include Mongoid::Fields::Serializable
  attr_accessor :name, :code

  FILTERS = ["Afghanistan","Brunei","Iran", "Kuwait", "Libya", "Saudi Arabia", "Sudan", "Yemen", "Britain (UK)", "Antarctica", "Bonaire Sint Eustatius & Saba", "British Indian Ocean Territory", "Cocos (Keeling) Islands", "St Barthelemy", "St Martin (French part)", "Svalbard & Jan Mayen","Vatican City"]

  EXTRAS = {
    'eng' => 'England',
    'wal' => 'Wales',
    'sco' => 'Scotland',
    'nlr' => 'Northern Ireland'
    }

  def initialize(name, code)
    @name = name
    @code = code
  end

  def deserialize(object)
    return nil unless object
    Country.new(object['name'], object['code'])
  end

  def serialize(country)
    {:name => country.name, :code => country.code}
  end

  def self.all
    add_extras(filter(TZInfo::Country.all.map{|country| to_country country})).sort! {|c1, c2| c1.name <=> c2.name}
  end

  def self.get(code)
    begin
      to_country TZInfo::Country.get(code)
    rescue TZInfo::InvalidCountryCode => e
      'InvalidCountryCode' unless EXTRAS.has_key? code
      Country.new EXTRAS[code], code
    end
  end

  def self.get_by_name(name)
    all.select {|country| country.name.downcase == name.downcase}.first
  end

  def self.filter(countries)
    countries.reject {|country| FILTERS.include?(country.name)}
  end

  def self.add_extras(countries)
    countries + EXTRAS.map{|k,v| Country.new v, k}
  end

  private
  def self.to_country(country)
    Country.new country.name, country.code
  end
end

and my request for the array which is called from another class

  def countries_ive_drunk
    (had_drinks.map {|drink| drink.beer.country }).uniq
  end

If I throw the array I can see the structure is:

[
#<Country:0x5e3b4c8 @name="Belarus", @code="BY">, 
#<Country:0x5e396e0 @name="Britain (UK)", @code="GB">, 
#<Country:0x5e3f350 @name="Czech Republic", @code="CZ">, 
#<Country:0x5e3d730 @name="Germany", @code="DE">, 
#<Country:0x5e43778 @name="United States", @code="US">, 
#<Country:0x5e42398 @name="England", @code="eng">, 
#<Country:0x5e40f70 @name="Aaland Islands", @code="AX">, 
#<Country:0x5e47978 @name="England", @code="eng">, 
#<Country:0x5e46358 @name="Portugal", @code="PT">, 
#<Country:0x5e44d38 @name="Georgia", @code="GE">, 
#<Country:0x5e4b668 @name="Germany", @code="DE">, 
#<Country:0x5e4a2a0 @name="Anguilla", @code="AI">, 
#<Country:0x5e48c98 @name="Anguilla", @code="AI">
]

This is the same, whether or not I do .uniq and you can see there is two "Anguilla"

Upvotes: 2

Views: 3840

Answers (6)

ChrisPhoenix
ChrisPhoenix

Reputation: 1090

At least as early as 1.9.3, Array#uniq would take a block just like uniq_by. uniq_by is now deprecated.

Upvotes: 0

Marc-Andr&#233; Lafortune
Marc-Andr&#233; Lafortune

Reputation: 79562

As pointed out by others, the problem is that uniq uses hash to distinguish between countries and that by default, Object#hash is different for all objects. It will also use eql? in case two objects return the same hash value, to be sure if they are eql or not.

The best solution is to make your class correct in the first place!

class Country
  # ... your previous code, plus:

  include Comparable

  def <=>(other)
    return nil unless other.is_a?(Country)
    (code <=> other.code).nonzero? || (name <=> other.name)
    # or less fancy:
    #   [code, name] <=> [other.code, other.name]
  end

  def hash
    [name, code].hash
  end

  alias eql? ==
end

Country.new("Canada", "CA").eql?(Country.new("Canada", "CA")) # => true

Now you can sort arrays of Countries, use countries as key for hashes, compare them, etc...

I've included the above code to show how it's done in general, but in your case, you get all this for free if you subclass Struct(:code, :name)...

class Country < Stuct(:name, :code)
  # ... the rest of your code, without the `attr_accessible` nor the `initialize`
  # as Struct provides these and `hash`, `eql?`, `==`, ...
end

Upvotes: 5

Frederick Cheung
Frederick Cheung

Reputation: 84114

This boils down to what does equality mean? When is an object a duplicate of another? The default implementations of ==, eql? just compare the ruby object_id which is why you don't get the results you want.

You could implement ==, eql? and hash in a way that makes sense for your class, for example by comparing the countries' codes.

An alternative is to use uniq_by. This is an active support addition to Array, but mongoid depends on active support anyway, so you wouldn't be adding a dependency.

some_list_of_countries.uniq_by {|c| c.code}

Would use countries' codes to uniq them. You can shorten that to 

 some_list_of_countries.uniq_by(&:code)

Upvotes: 2

Victor Deryagin
Victor Deryagin

Reputation: 12225

Objects in array are considered duplicate by Array#uniq if their #hash values are duplicate, which is not the case in this code. You need to use different approach to do what intended, like this:

def countries_ive_drunk
  had_drinks.map {|drink| drink.beer.country.code }
    .uniq
    .map { |code| Country.get code}
end

Upvotes: 2

Alex Kurkin
Alex Kurkin

Reputation: 1069

#<Country:0x5e4a2a0 @name="Anguilla", @code="AI">, 
#<Country:0x5e48c98 @name="Anguilla", @code="AI">

Array#uniq thinks these are different objects (different instances of Country class), because the objects' ids are different. Obviously you need to change your strategy.

Upvotes: 0

rohit89
rohit89

Reputation: 5773

Each element in the array is separate class instance.

#<Country:0x5e4a2a0 @name="Anguilla", @code="AI"> 
#<Country:0x5e48c98 @name="Anguilla", @code="AI">

The ids are unique.

Upvotes: 0

Related Questions