def phone_attributes=(phone_attributes)
phone_attributes.each do |attributes|
if attributes[:id].blank?
phones.build(attributes)
else
phone = phones.detect { |t| t.id == attributes[:id].to_i }
phone.attributes = attributes
end
end
end
def save_phones
phones.each do |t|
if t.should_destroy?
t.destroy
else
t.save(false)
end
end
end
def email_attributes=(email_attributes)
email_attributes.each do |attributes|
if attributes[:id].blank?
emails.build(attributes)
else
email = emails.detect { |e| e.id == attributes[:id].to_i }
email.attributes = attributes
end
end
end
def save_emails
emails.each do |e|
if e.should_destroy?
e.destroy
else
e.save(false)
end
end
end
Refactorings
No refactoring yet !
Brian
October 16, 2007, October 16, 2007 21:22, permalink
2 ratings.
Login to rate!
Refactored!
%w{phone email}.each do |prop|
src = <<-END_SRC
def #{prop}_attributes=(#{prop}_attributes)
#{prop}_attributes.each do |attributes|
if attributes[:id].blank?
#{prop.pluralize}.build(attributes)
else
#{prop} = #{prop.pluralize}.detect{ |t| t.id == attributes[:id].to_i}
#{prop}.attributes = attributes
end
end
end
def save_#{prop}
#{prop.pluralize}.each do |t|
if t.should_destroy?
t.destroy
else
t.save(false)
end
end
end
END_SRC
class_eval src, __FILE__, __LINE__
end
Etandrib
October 16, 2007, October 16, 2007 21:56, permalink
No rating.
Login to rate!
Thanks for this. I think I understand it a little better. I need to look into the class_eval stuff you've done here. I haven't seen that before.
Emmett
October 17, 2007, October 17, 2007 01:12, permalink
4 ratings.
Login to rate!
You don't need eval to do this; I'm generally opposed to using it unless you actually can't get what you want any other way reasonably. There's a tiny amount of duplication in exchange for simplified code.
def set_model_attributes(model_name, model_attributes)
model_attributes.each do |attributes|
if attributes[:id].blank?
send(name).build(attributes)
else
model = send(name).detect {|t| t.id == attributes[:id].to_i}
model.attributes = attributes
end
end
end
def save_models(name)
send(name).each do |m|
if m.should_destroy?
m.destroy
else
m.save(false)
end
end
end
def save_phones() save_models(:phones); end
def save_emails() save_models(:emails); end
def email_attributes=(attributes) set_model_attributes(:emails, attributes); end
def phone_attributes=(attributes) set_model_attributes(:phones, attributes); end
Brian
October 17, 2007, October 17, 2007 02:21, permalink
No rating.
Login to rate!
I think that's a good start for a plugin. Just create one method that defines both method and throw it in a lib directory ad you're good.
Mike
December 4, 2007, December 04, 2007 02:13, permalink
No rating.
Login to rate!
One small modification to the above posted code: Changed "model_name" argument for the set_model_attributes method to "name" otherwise error will be thrown for name being undefined. I like the idea, works well!
def set_model_attributes(name, model_attributes)
model_attributes.each do |attributes|
if attributes[:id].blank?
send(name).build(attributes)
else
model = send(name).detect {|t| t.id == attributes[:id].to_i}
model.attributes = attributes
end
end
end
def save_models(name)
send(name).each do |m|
if m.should_destroy?
m.destroy
else
m.save(false)
end
end
end
def save_phones() save_models(:phones); end
def save_emails() save_models(:emails); end
def email_attributes=(attributes) set_model_attributes(:emails, attributes); end
def phone_attributes=(attributes) set_model_attributes(:phones, attributes); end
We have a model where a person has many phones, emails, addresses, etc. It would be nice to refactor these into something more compact. I'm basically replicating the "model_attributes" and "save_model" functions over and over for each piece of contact information I add to a user.