I have identical getter/setter code used for multiple virtual attributes - can I refactor with eval?

Question

I have written customised getter and setter methods for virtual attributes to convert decimals into integers for storage in a database. This is one of three virtual attributes (annual_fee_dollars) that get/sets from a real attributes (annual_fee) in the database:

def annual_fee_dollars
  @annual_fee_dollars || int_to_dec(annual_fee)
end

def annual_fee_dollars=(string)
  @annual_fee_dollars = string
  self.annual_fee = dec_to_int(string)
end

Instead of repeating all this code three times, does it make sense to / is it safe to / is it the 'Rails Way' to refactor the code like this:

def self.decimal_get_and_set(variable, suffix)
  eval (
    "def #{variable + suffix}
      @#{variable + suffix} || int_to_dec(self.#{variable})
    end
    def #{variable+suffix}=(string)
      @#{variable+suffix} = string
      self.#{variable} = dec_to_int(string)
    end")
end
self.decimal_get_and_set "annual_fee", "_dollars"
self.decimal_get_and_set "interest_purchase", "_percent"
self.decimal_get_and_set "interest_cash", "_percent"

Or is there a much cleaner way to build this type of functionality?

Apologies if this is a 'subjective question'. In part, all refactoring questions have some subjectivity to them, but I think this question still has a place on SO. Happy to be corrected on this.

Cheers!

Answer 1

I think your approach is fine, but I wouldn't suggest using eval, mainly because there is already a more appropriate ruby metaprogramming way to do this. Read up on the documentation for define_method and the object methods instance_variable_get and instance_variable_set.

Looks like what you want and you don't need to use eval yourself. I would probably suggest something like the following, but you're right - all refactoring questions are somewhat subjective by their very nature. Good luck!

{'annual_fee' => '_dollars', 'interest_purchase' => '_percent', 'interest_cash' => '_percent'}.each_pair do |variable, suffix|
  # Define getters
  define_method "#{variable+suffix}" do
    instance_variable_get("@#{variable+suffix}") || int_to_dec(send("#{variable}")
  end

  # Define setters
  define_method "#{variable+suffix}=" do
    ...
  end
end