Topic: How to make this DRY?

Hi Community!

I've got the following code in 2 models and would like to make it "dry"er... how should i do that?

many thanks,
maze

# clip model
has_many :permissions

  def add_readers(users)
    users.each do |user|
      self.permissions.add_reader(user,self)
    end
  end

  def add_writers(users)
    users.each do |user|
      self.permissions.add_writer(user,self)
    end
  end

  def add_deleters(users)
    users.each do |user|
      self.permissions.add_deleter(user,self)
    end
  end


# permissions model
belongs_to :clip

  def self.add_reader(user, clip)
    permission = self.find_existing(user,clip) || self.new(:clip_id => clip.id, :user_id => user.id)
    permission.update_attributes(:read => true)
  end

  def self.add_writer(user, clip)
    permission = self.find_existing(user,clip) || self.new(:clip_id => clip.id, :user_id => user.id)
    permission.update_attributes(:write => true)
  end

  def self.add_deleter(user, clip)
    permission = self.find_existing(user,clip) || self.new(:clip_id => clip.id, :user_id => user.id)
    permission.update_attributes(:delete => true)
  end

Re: How to make this DRY?

def self.build_add_methods_for(*attrs)
    attrs.each do |attr|
      eval "def add_#{attr}s(users); users.each do |user| ; self.permissions.add_#(attr}(user,self);;"
    end
  end

build_add_methods_for :reader, :writer, :deleter


Just recently learned this trick thanks to vwoo. Untested but should work

Re: How to make this DRY?

Also, the lines that look like this:

permission = self.find_existing(user,clip) || self.new(:clip_id => clip.id, :user_id => user.id)

can be refactored to:

permission = find_or_initialize_by_user_id_and_clip_id(user.id, clip.id)

Not a whole bunch shorter, but definitely more readable. Also, if this is the only place you use it, you can get rid of the find_existing method