2006/4/10, Mark Alexander Friedgan <hubrix / gmail.com>:
> I wrote some code that searches ObjectSpace prior to initializing an
> object to see if one of that type and some attributes is already
> present and uses that instead of initializing. (yes it's active record
> and i am trying to avoid stale object errors). How horrible is this? I
> am searching by specific class name so in practice it ends up looking
> at at most a couple of instances but is there something inherently
> wrong with doing this?
>
> def find_in_space(search_id, search_version = nil)
>   retval =  nil
>   ObjectSpace.each(Person){|person|
>       retval = person if (p.id == search_id) &&
>                                search_version &&
>                                p.lock_version == search_version
>   }
>   return retval || Person.find(search_id)
> end

This is bound to be slow.  The easiest improvement you can do is to
have the return inside the block.  But even that will not change the
order of magnitude (both approaches are O(n)).

Also, you seem to have a bug in there - you frequently use "p." where
it should probably rather read "person.".

If you do those lookups frequently, I'd rather consider using an
approach that uses a hash inside the class object.

# example
class Person
  attr_reader :id
  def initialize(id) @id = id end

  @instances = Hash.new {|h,k| h[k] = [new(k)]}

  def self.get(search_id, search_version = nil)
    arr = @instances[search_id]
    arr.each {|per| return per if search_version == per.lock_version}
if search_version
    arr[0]
  end
end

I don't know what exactly you need the version for but the asymmetry
(with and without version) strikes me as odd.

Kind regards

robert