nagbibinata nagbibinata -4 years ago 61
Ruby Question

Rails - How to refactor repeated method call on attributes

class Cost < ApplicationRecord
before_save :convert

private
def convert #REFACTOR
self.val_1 = multiply_by_100(self.val1)
self.val_2 = multiply_by_100(self.val2)
self.val_3 = multiply_by_100(self.val3)
end

def multiply_by_100(number)
number*100 if number.present?
end
end


I have this
Cost
model with attributes val_1, val_2, and val_3.
I need to multiply these attributes by 100 before saving to database.
My implementation above works fine but it seems odd to look at.
Any recommendation to refactor it?

Updated: add if condition in multiply_by_100 method

Answer Source

My implementation above works fine but it seems odd to look at.

I disagree. It's dead simple, and this is an important quality of code. The only improvement I'd make is drop unnecessary self from reader calls.

self.val_1 = multiply_by_100(val1)

I'm guessing you were looking to DRY it up with some dynamic method invocations, like this:

class Cost < ApplicationRecord
  before_save :convert

  private
  ATTRS = ['val_1', 'val_2', 'val_3']

  def convert
    ATTRS.each do |attr|
      send("#{attr}=", multiply_by_100(send(attr)))
    end
  end
end

Me, I'd only use that if attribute set will change in the future (you add new attributes to convert, or something like that). If it's just the three attributes, I'd leave the code as-is.

Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download