How to write this Ruby function using a loop?

Question

Is there a more elegant way to write this in Ruby, maybe using a loop?

def save_related_info
  update_column(:sender_company_name, user.preference.company_name)
  update_column(:sender_address, user.preference.address)
  update_column(:sender_telephone, user.preference.telephone)
  update_column(:sender_email, user.preference.email)
  update_column(:sender_url, user.preference.url)
  update_column(:sender_vat_number, user.preference.vat_number)
  update_column(:sender_payment_details, user.preference.payment_details)
end

Thanks for any help.

We don't have enough code, but it looks like that model should have an association to "preference" to avoid such duplication of data, doesn't it? — tokland
– tokland, Commented Aug 10, 2013 at 12:07

Peter Brown · Accepted Answer · 2013-08-10 12:11:01Z

3

Any reason you're not using update_attributes to do them all at once?

def save_related_info
  update_attributes(
    :sender_company_name => user.preference.company_name,
    :sender_address => user.preference.address,
    :sender_telephone => user.preference.telephone,
    :sender_email => user.preference.email,
    :sender_url => user.preference.url,
    :sender_vat_number => user.preference.vat_number,
    :sender_payment_details => user.preference.payment_details
  )
end

answered Aug 10, 2013 at 12:11

Peter Brown

51.8k18 gold badges116 silver badges147 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

tokland Over a year ago

Note that update_column skips validations, callbacks and timestamps updating, so it's not the same.

Peter Brown Over a year ago

@tokland That sounds like a bad idea to me, but if the OP does infact want to skip that stuff, Rails 4 has update_columns

tokland Over a year ago

I agree, updating with pure SQL ignoring all the model stuff is usually a bad idea.

Tintin81 Over a year ago

Good point. I am still learning Ruby, so I just read up on the difference between the two.

sawa · Accepted Answer · 2013-08-10 11:56:57Z

2

def save_related_info
  %w[company_name address telephone email url vat_number payment_details]
  .each{|s| update_column("sender_#{s}".to_sym, user.preference.send(s))}
end

answered Aug 10, 2013 at 11:56

sawa

169k51 gold badges287 silver badges398 bronze badges

7 Comments

tokland Over a year ago

maybe shorter with :"sender_#{s}"? and definitely I'd use the standard indented do/end block, more clear just at the expense of 1 line.

sawa Over a year ago

@tokland Regarding the symbol literal, it's the same. Putting a method and its block together in one line makes it look easier, I think.

silent_coder Over a year ago

Don't think that build column name from property name it's a good idea. In that example it could be correct but in general could cause a lot of problems and possible errors.

tokland Over a year ago

@silent_coder why? that seems like the DRY way of doing it, and from the question is clear there is a 1:1 correspondence, I don't see the problem. If some attribute changes then you create a hash with the exceptions, it's very typical stuff.

tokland Over a year ago

If columns and properties did not match then I'd write a column => property hash. But if they all match I won't lose a second with it. Write the simplest code that works, update only when necessary.

|

Ph0en1x · Accepted Answer · 2013-08-10 15:40:04Z

1

first guess is to put keys in and values in lists and then use loop. something like this:

keys = ['key1', 'key2', 'key3', 'key4']
values = [val1, val2, val3, val4]
keys.each_index do |i|
 update_column(keys[i], values[i])
end

Minus in that approach is that the order of elements in values array should fit for order of keys. You could avoid it of using hash instead of arrays. Code will looks like this:

data = { "key1" => val1, "key2" => val2, "key3" => val3 };
data.each do |key, value|
 update_column(keys, values)
end

edited Aug 10, 2013 at 15:40

answered Aug 10, 2013 at 11:53

Ph0en1x

10.1k8 gold badges55 silver badges97 bronze badges

6 Comments

tokland Over a year ago

note that you'd use zip in the first snippet if you decided to write it this way.

Ph0en1x Over a year ago

I know that it will be better from ruby style point of view, but I'm coding multiple languages and I prefer to have solid coding style which is the same for most of the languages. But I'm agree, that in Ruby case your better will be used %w notation.

tokland Over a year ago

Ok, but personally I prefer to write the most idiomatic code in every language (btw, a lot of languages have zip: haskell, python, scala, erlang, JS+underscore...).

sawa Over a year ago

@Ph0en1x If you keep the same style of coding for different languages, then that defeats the purpose of having different languages. Why do you think there are different languages?

Ph0en1x Over a year ago

Because clients need different languages. And in many cases I can't choose, just use what they ask.

|

Collectives™ on Stack Overflow

How to write this Ruby function using a loop?

3 Answers 3

4 Comments

7 Comments

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

7 Comments

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related