Ruby setter idiom

Question

I'm working on a Chart class and it has a margin parameter, that holds :top, :bottom, :right and :left values. My first option was to make margin a setter and set values like this:

# Sets :left and :right margins and doesn't alter :top and :bottom 
chart.margins = {:left => 10, :right => 15}

It's nice, because it is clearly a setter, but, after some thought, I think it could be confusing too: the user might think that margins contains only :left and :right values, what is not right. Another option is eliminate = and make it an ordinary method:

chart.margins(:left => 10, :right => 15)

With this syntax, it's easy to figure out what is happening, but it is not a standard setter and conflicts with margins getter. And there's still another option:

chart.margins(:left, 10)
chart.margins(:right, 15)

I don't know what to think about this. For me, it is obvious the method is a setter, but this time I cannot set multiple values with just one call and there's the problem with getter again. I'm relatively new to Ruby and I haven't got used to all the idioms yet. So, what do you think guys? Which is the best option?

Answer 1

You could also make a Margin class to enjoy the following clear syntax:

class Margin
    attr_accessor :left, :right, :top, :bottom
    ...
end

class Chart
    attr_accessor :margins
    ...
 end


chart.margins.left = 10
puts chart.margins.right

Answer 2

In addition to paradigmatic's answer, you could add a method to the Margins class to support:

chart.margins.set :left => 10, :right => 15

You could extend the margins= method to treat a numeric argument:

chart.margins = 20

as sugar for:

chart.margins = Margins.new(20, 20, 20, 20)

Answer 3

Not so sure if this is the kind of syntax that you would like to make available (sorry if not : )

#!/usr/bin/ruby
class Margins < Struct.new(:top, :bottom, :left, :right) 
end

class Chart
  attr_reader :margins

  def initialize()
    @margins = Margins.new(0,0,0,0)
  end

  def margins=(hash)
    [:top, :bottom, :left, :right].each do |dir|
      if (hash[dir])
        @margins[dir] = hash[dir]
      end
    end
  end
end

c = Chart.new
c.margins.left = 10
c.margins={:top=>12,:bottom=>13}
puts c.margins.left
# 10
puts c.inspect;
# #<Chart:0xb7caaf8c @margins=#<struct Margins top=12, bottom=13, left=10, right=0>>

# However c.margins.foo = 12 would give you an error

Answer 4

I don't think creating a class for Margin is an overkill. You can always expose its values as a hash using to_hash or something similar.

Also, if you like, you can make it work in DSL-style:

chart.margins do |m|
  m.left 10
  m.right 20
  m.vertical 5 # sets both top and bottom margin
end

But I guess I would choose paradigmatic's approach anyway...

Answer 5

You could also stick with what you had first and use the normal hash syntax.

margins["left"] = 10  #to set just one without changing the others