From: Hugh Sasse <hgs@...>
Date: 2005-09-14T02:38:07+09:00
Subject: Re: Sets, uniqueness not unique.

On Wed, 14 Sep 2005, Ara.T.Howard wrote:

> On Wed, 14 Sep 2005, Hugh Sasse wrote:
>
>> require 'set'
>> 
>> class Student
>>  attr_accessor :forename, :surname, :birth_dt,
>>    :picture, :coll_status
>>  def initialize(forename0, surname0, birth_dt0,
         [...]
>>  end
>>
>>  def eql?(other)
         [...]
>>  end
>>
>>  def to_s
>>    "#{@surname}, #{@forename}, #{@birth_dt}, #{@picture}, #{@coll_status}"
>>  end
>> end
>
> well this works:
>
>  s0 = Student::new 'a', 'b', 'c', 'd', 'e'
>  s1 = Student::new 'a', 'b', 'c', 'd', 'e'
>  p(s0.eql?(s1))  #=> true
>
> but this doesn't
>
>  p s0 == s1  #=> false

Hmmm.  Yes, I should have more unit tests!
>
>> And in the body of my program I read the records in from the csv and

(well. pipe separated -- see other reply :-))

>> add the students if they are new. They tend to be clustered in the
>> input, hence the last_student test.
>> 
>> class TableMaker
         [...]
>>  def initialize(input=INPUT)
         [...]
>>    open(input, 'r') do |infp|
>>      while record = infp.gets
>>        record.chomp!
>
> try : record.strip!

>
>>        puts "record is #{record}"
>>        forename, surname, birth_dt, institution_id, aos_code,
>>          various, other, fields,
>>          picture, coll_status, full_desc = record.split(/\s*\|\s*/)
>
> or
>      fields = record.split(%r/\|/).map{|field| field.strip}
>      forename, surname, birth_dt, institution_id, aos_code,
>      various, other, fields,
>      picture, coll_status, full_desc =

I think the former may be faster, but I'll look into these, thanks.
>
>
> if you don't do one of these two things the either
>
>  - forname may have leading space
>  - full_desc may have trailing space

Yes, I'd missed that.
>
> that's because chomp! only blows away trailing newline - not extraneous
> spaces and leading space on record is never dealt with.
>
>>
>>        next unless aos_code =~ ACCEPTED_MODULES
>>
>>        puts "from record, picture is [#{picture.inspect}]." if $debug
>>        # Structures for student
>>        student = Student.new(forename, surname, birth_dt, picture, 
>> coll_status)
>>        if student == last_student
>
> so, as shown above, this (==) does not work

OK, I'll just lose optimisation, but thanks.
>
>>          student = last_student
>>        else
>>          student.freeze
>>
>>          # Avoid duplicates
>>          unless @students.include? student
>>            @students.add student
>>          end
>>          last_student = student
         [...]
>> 
>> This being a Set I don't really need the call to include? now, but
>> it's there (from when I was using a hash for this).
>> 
>> I find two things that seem odd to me:
>> 
>> 1. eql? is never getting called, despite include?.
>
> set uses Object#hash - so maybe something like (untested)
>
>  class Student
>    def hash
>      %w( forename surname birth_dt picture coll_status).inject(0){|n,m| n += 
> send(m).hash}
>    end
>  end
>
> i dunno if this will wrap and cause issues though...

Nor me.
>
> if so maybe something like
>
>  class Student
>    def hash
>      %w( forename surname birth_dt picture coll_status).map{|m| send 
> %m}.join.hash
>    end
>  end

Yes, that seems safer
>
> or, perhaps simple something like:
>
>  class Student < ::Hash
>    FIELDS = %w( forename surname birth_dt picture coll_status )
         [...]
>  end
>
>  s0 = Student::new 'a', 'b', 'c', 'd', 'e'
>  s1 = Student::new 'a', 'b', 'c', 'd', 'e'
>
>  require 'set'
>  set = Set::new
>  set.add s0
>  set.add s1
>  p set #=> #<Set: {{"forename"=>"a", "coll_status"=>"e", "birth_dt"=>"c", 
> "picture"=>"d", "surname"=>"b"}}>
>
> the FIELDS const can be used to do ordered prints, etc.

Yes, I might factor that in to my current solution.  I didn't want
to allow just any keys, so that's why I didn't subclass Hash, but
it's an interesting approach.
>
> it sure seems odd that set doesn't use 'eql?' or '==' up front though doesn't
> it?

Probably a reason I don't know about.  The Pickaxe II says it uses
eql? and hash (p731) but doesn't say where. 
>
> -a
> --

         Thank you for such a full response,
         Hugh.