views:

58

answers:

1

A user has many employments.

What do you think? Is this a valid and clear way to fetch all siblings (belonging to the same user) of a given employment object?

class Employment < ActiveRecord::Base
  belongs_to :user

  has_many :silblings,
    :primary_key => :user_id,
    :foreign_key => :user_id,
    :class_name => 'Employment'
end

This can be extended with the following named scope:

  named_scope :except, lambda {|id| {:conditions => ["id != ?", id]} if id}

Now I can do stuff like:

  self.silblings.except(self.id).each do |silbling|
    puts silbling
  end

The resulting SQL statement looks like:

  SELECT * FROM `employments` 
  WHERE (`employments`.user_id = 49) 
  AND ((id != 46) AND (`employments`.user_id = 49))

Comments like 'no, you abuse XY, rather use this XZ' are very welcome!

Reto

+3  A: 

Looks fine. Except that the SQL doubles ('employments'.user_id = 49) in the query. Which is nothing major. If it's something you really don't want, you could go about defining siblings like this:

class Employment < ActiveRecord::Base
  belongs_to :user

  named_scope :for_user, lambda { |user|
    { :conditions => {:user_id => user} }
  }

  named_scope :except, lambda {|employment|
    {:conditions => ["id != ?", employment}
  }

  def siblings 
    Employment.for_user(user_id).except(id)
  end

end

Believe it or not you can still call named scopes on @employment.siblings. Although doing things this way means you can't assign to siblings. The siblings call comes out a little cleaner. There may be a performance improvement, but it probably won't be significant to make a difference.

EmFi
for_user looks much cleaner, simpler and nicer. It's less generic, but much more explicit. I think I'll go for it. The duplication is an unrelated bug. I found the corresponding bug report shortly after I posted the question: https://rails.lighthouseapp.com/projects/8994/tickets/2923Thanks for your input.
reto
thank you EmFi!
reto