1

I'm defining a Box class in ruby which has 3 instance variables: @length, @width, @height, and 2 class variables: @@boxcounter and @@totalvolume.

While the value of @@boxcounter can be updated inside the object constructor (initializer), updating the value of @@totalvolume becomes less trivial, since we have to recalculate the volume of the given object each time we change any of the instance variables (i.e. length, width, or height).

I've come up with the following code to handle this use case:

class Box
  @@boxcounter = 0
  @@totalvolume = 0.0

  def initialize(length, width, height)
    @length = length
    @width = width
    @height = height
    @@boxcounter += 1
    @@totalvolume += volume
  end

  def volume
    volume = @length * @width * @height
  end

  def report
    puts "# of boxes: #{@@boxcounter}"
    puts "Total volume: #{@@totalvolume}"
    puts "Average volume per box: #{@@totalvolume / @@boxcounter}"
  end

  def my_print
    p self
    puts "Length: #{@length}"
    puts "Width: #{@width}"
    puts "Height: #{@height}"
    puts
  end

  def length=(length)
    @@totalvolume -= volume
    @length = length
    @@totalvolume += volume
  end

  def width=(width)
    @@totalvolume -= volume
    @width = width
    @@totalvolume += volume
  end

  def height=(height)
    @@totalvolume -= volume
    @height = height
    @@totalvolume += volume
  end
end

Since the idea of first class objects is still in my head after studying Scheme, I wondered, can I create a generic setter and use that to reduce the code repetition in each of the setters listed above? I tried this, but using eval seems to be a bit of a hack:

  def update_class_state(update_instance_variable)
    @@totalvolume -= volume
    eval update_instance_variable
    @@totalvolume += volume
  end

  def length=(length)
    update_class_state("@length = #{length}")
  end

  def width=(width)
    update_class_state("@width = #{width}")
  end

  def height=(height)
    update_class_state("@height = #{height}")
  end

– My question: Is it a bad practice to write code like this? Is there a more elegant solution to this approach?

lacostenycoder
  • 10,623
  • 4
  • 31
  • 48
wintermute
  • 488
  • 1
  • 9
  • 22
  • Skimming through your code it looks like you're looking for [attr_accessor and co](https://stackoverflow.com/questions/4370960/what-is-attr-accessor-in-ruby). – Sagar Pandya May 08 '18 at 01:24

1 Answers1

1

There's nothing inherently "wrong" with your approach except the use of eval. Here's a more dynamic way to remove the duplication without needing use of eval

class Box
  @@boxcounter = 0
  @@totalvolume = 0.0

  def initialize(length, width, height)
    @length = length
    @width = width
    @height = height
    @@boxcounter += 1
    @@totalvolume += volume
  end

  def volume
    volume = @length * @width * @height
  end

  def report
    puts "# of boxes: #{@@boxcounter}"
    puts "Total volume: #{@@totalvolume}"
    puts "Average volume per box: #{@@totalvolume / @@boxcounter}"
  end

  def my_print
    p self
    puts "Height: #{@height}"
  end

  # dynamically define your methods to dry up the code:
  %w[length width height].each do |method|
    define_method("#{method}=") do |arg|
      @@totalvolume -= volume
      instance_variable_set('@'+method, arg)
      @@totalvolume += volume
    end
  end
end
lacostenycoder
  • 10,623
  • 4
  • 31
  • 48
  • Thank you for your reply, that is a very interesting approach. – wintermute May 08 '18 at 03:07
  • 1
    @wintermute no problem. Whenever you see code that is identical besides say the name of a variable, you can almost always use some kind of dynamic ruby methods and string interpolation to dry things up. One downside of this approach however is if you have a large code base and are searching for a method definition, you might not find it with a full text search. In those cases it may be worth adding comments with the generated method names. – lacostenycoder May 08 '18 at 03:12