CODEWITHSUNDEEP
ruby-on-railsrefactoringruby-on-rails-3
Yuval Karmi
Yuval Karmi

Reputation: 26713

How can I refactor this Rails 3 code?

Can anybody figure a way to refactor this further?

@hourly_pay = {}
HourlyPay.all.each { |hp| @hourly_pay[t("hourly_pay.#{hp.amount}")] = hp.amount }

Thanks!

Edit: based on the answers I've received, here's how I refactored

HourlyPay.all.map(&:amount).index_by { |hp| t("hourly_pay.#{hp.amount}") }

Placing this into my model directly, this becomes

  def self.get_options
   all.map(&:amount).index_by { |hp| I18n.t("hourly_pay.#{hp.amount}") }
  end

However, I'm not sure if this is more calculation intensive, since I'm calling map on the values returned from the database, and then calling index_by on that. Since my HourlyPay model only includes an id and an amount, I'm not worried about selecting everything. However, if I had a lot more fields, I would do the following instead:

def self.get_options
 select(:amount).map(&:amount).index_by { |hp| I18n.t("hourly_pay.#{hp.amount}") }
end

So only the amount field is selected

Thanks for the responses!

Upvotes: 2

Views: 209

Answers (1)

Aditya Sanghi
Aditya Sanghi

Reputation: 13433

The intention is not very clear here but I suggest that you look at using the index_by method in enumerable class. You'll get the complete HourlyPay object as the value in the hash though and can get the hash in one line.

Consider putting this code in the model if not already there.

Upvotes: 2

Related Questions