From: Robert Klemme <shortcutter@...>
Date: 2008-11-03T17:11:38+09:00
Subject: Re: Indexed arrays, delete_if, and performance

2008/11/3 Jason Leong <jason@pocketsmith.com>:
>  > Please look carefully at the code.  Although untested it should do
>> exactly this: delete from the other Hash.  Note that the return value of
>> Hash#delete is the deleted element.  I probably should have added a
>> safety check to ensure nil does not cause errors.  So you'd probably
>> rather do
>>
>>    def delete_by_id(id)
>>      dlt = @ev_by_id.delete(id) and dlt.each do |ev|
>>        @ev_by_date[ev.date].delete(ev)
>>      end
>>    end
>
> Ah yes! Pounded off a reply before I looked, sorry - thanks for the
> safety check too, that certainly came in handy.

And it's needed if someone passes in a wrong id or date ("wrong"
meaning, there is no data for it).

> The one difference in my
> final implementation is this:
>
>  def delete_events_by_id(id)
>    dlt = @titles.delete(id.to_i) and dlt.each do |e|
>      @events[e.date].delete_if { |i| i.id == e.id }
>    end
>  end
>
> The reason being (I think!) the object passed in for deletion in
> @ev_by_date[ev.date].delete(ev) does not match up with the object in
> @ev_by_date as they're two different Hashes - but a compare based on the
> id works. Does that sound right?

I do not know the rest of your code but in my version the same
instance was put into both Hashes.  If you think about it, this is
what you want, i.e. regardless of whether you look up the event by id
or date you want to have the same instance - otherwise you would have
to manipulate two instances all the time, which makes code more
complex and also wastes memory. And in that case Array#delete is
sufficient:

irb(main):001:0> o = Object.new
=> #<Object:0x7ff9c69c>
irb(main):002:0> a = [o]
=> [#<Object:0x7ff9c69c>]
irb(main):003:0> a.delete o
=> #<Object:0x7ff9c69c>
irb(main):004:0> a
=> []

Even for multiple occurrences:

irb(main):005:0> a = [o,o]
=> [#<Object:0x7ff9c69c>, #<Object:0x7ff9c69c>]
irb(main):006:0> a.delete o
=> #<Object:0x7ff9c69c>
irb(main):007:0> a
=> []

But delete_if is ok of course as well. If you need more efficiency you
can use Set instead of Array as Hash values but then you should use
#delete because this is more efficient for Set (O(1) vs. O(n) for
delete_if).

> Thanks Robert, you've taught me a bunch about Hashes!

You're welcome!

Kind regards

robert


-- 
remember.guy do |as, often| as.you_can - without end