0

I try to optimize the following Accounting model class and have two questions:

1) How can i replace the multiple attribute setters with a more elegant method?

2) Is there a better way than the if conditional in the replace_comma_with_dot method

class Accounting < ActiveRecord::Base

  validates :share_ksk, :central_office, :limit_value, :fix_disagio,
            presence: true, numericality: { less_than: 999.99, greater_than_or_equal_to: 0.00 },
            format: { with: /\d*\.\d{0,2}$/, multiline: true, message: I18n.t('accounting.two_digits_after_decimal_point')}

  def share_ksk=(number)
    replace_comma_with_dot(number)
    super
  end

  def central_office=(number)
    replace_comma_with_dot(number)
    super
  end

  def limit_value=(number)
    replace_comma_with_dot(number)
    super
  end

  def fix_disagio=(number)
    replace_comma_with_dot(number)
    super
  end

  def replace_comma_with_dot(number)
    if number.is_a? String
      number.sub!(",", ".")
    elsif number.is_a? Float
      number
    else
      ""
    end
  end

end

As user Pardeep suggested i'm trying to replace my getters with define_method:

  [:share_ksk=, :central_office=, :limit_value=, :fix_disagio=].each do |method_name|
    self.class.send :define_method, method_name do |number|
      replace_comma_with_dot(number)
      super
    end
  end

What am i missing?

StandardNerd
  • 4,093
  • 9
  • 46
  • 77

2 Answers2

3

You can define methods dynamically using the define_method, you can fine more information here

and you can update your replace_comma_with_dot with this

def replace_comma_with_dot(number)
  return number.sub!(",", ".") if number.is_a? String
  return number if number.is_a? Float
  ""
end

end

Pardeep Saini
  • 2,084
  • 1
  • 18
  • 29
1

Instead of having a method in your model, I'd extract the functionality and append it to either the String or Integer classes:

#lib/ext/string.rb
class String
  def replace_comma_with_dot
    number.sub!(",",".") #Ruby automatically returns so no need to use return
  end
end

This - if your number is a string will allow you to do the following:

number = "67,90"
number.replace_comma_with_dot

To use this in the app, the setters are okay. You could achieve your functionality as follows:

 def fix_disagio=(number)
    self[:fix_disagio] = number.replace_comma_with_dot
 end

Your update is okay, except I would shy away from it myself as it creates bloat which doesn't need to be there.

I was looking for a way to set the attributes when you pull from the db, but then I realized that if you're having to set this each time you call the model, surely something will be wrong.

I would personally look at changing this at db level, failing that, you'd probably be able to use some sort of localization to determine whether you need a dot or comma.

There is a good answer here which advocates adding to the ActiveRecord::Base class:

class ActiveRecord::Base
  def self.attr_localized(*fields)
    fields.each do |field|
      define_method("#{field}=") do |value|
        self[field] = value.is_a?(String) ? value.to_delocalized_decimal : value
      end
    end
  end
end

class Accounting < ActiveRecord::Base
  attr_localized :share_ksk
end
Community
  • 1
  • 1
Richard Peck
  • 76,116
  • 9
  • 93
  • 147