Idiomatic Ruby

Posted by Jamis Friday, December 08, 2006 05:23:00 GMT

Dan Hancu sent us a nice, bite-sized snippet of code, asking if there was a more elegant way to do what he was doing. His code was from a controller, part of a larger application that included a system for tracking an inventory of lenses. When creating a new type of lens, you just input a few parameters and the system then autogenerates all relevant lenses for that type of lens.

His code (in essense) is:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33

class TypeOfLensController < ApplicationController
  def create
    @type_of_lens = TypeOfLens.new(params[:type_of_lens])
    if @type_of_lens.save
      flash[:notice] = 'TypeOfLens was successfully created.'
      populate(@type_of_lens)
      redirect_to :action => 'list'
    else
      render :action => 'new'
    end
  end

  private
  
    def populate(type_of_lens)
      x_count = type_of_lens.minimum_cylinder
      y_count = type_of_lens.maximum_sphere
      increment_step_size = 0.25
      while y_count >= type_of_lens.minimum_sphere
        while x_count <= type_of_lens.maximum_cylinder
          lens = Lens.new
          lens.lens_type_id = type_of_lens.id
          lens.sphere = y_count
          lens.cylinder = x_count
          lens.quantity = 0
          lens.save
          x_count += increment_step_size
        end
        x_count = type_of_lens.minimum_cylinder
        y_count -= increment_step_size
      end
    end
end

Now, naturally, since I’m not acquainted with the domain of the application in question, it’s hard for me to judge the bigger picture, such as whether the model associations and such are appropriate or not. I’m going to assume that Dan, who is much more acquainted with the domain than myself, did all that right, and I’ll just focus on some of the smaller mechanical details of the code in question.

First off, the populate method really ought to be a method on TypeOfLens, called via an after_create hook:

1
2
3
4
5
6
7

class TypeOfLens < ActiveRecord::Base
  after_create :populate

  def populate
    #...
  end
end

By setting up populate as an after_create hook, we can rest easily knowing that every time a new TypeOfLens is instantiated, that populate method will be auto-invoked. That knowledge lets us simplify the controller significantly:

1
2
3
4
5
6
7
8
9
10
11

class TypeOfLensController < ApplicationController
  def create
    @type_of_lens = TypeOfLens.new(params[:type_of_lens])
    if @type_of_lens.save
      flash[:notice] = 'TypeOfLens was successfully created.'
      redirect_to :action => "list"
    else
      render :action => "new"
    end
  end
end

Now, we can turn our attention to the populate method itself. Although Dan’s original code certainly suffices, it has the flavor of C code, simply modified to use basic Ruby syntax. By taking advantage of standard Ruby methods and patterns, we can not only compact the code, we can also make it look like idiomatic Ruby code instead of C code.

First of all, note the loop idiom being used:

1
2
3
4
5
6
7
8
9
10
11
12
13

def populate(type_of_lens)
  x_count = type_of_lens.minimum_cylinder
  y_count = type_of_lens.maximum_sphere
  increment_step_size = 0.25
  while y_count >= type_of_lens.minimum_sphere
    while x_count <= type_of_lens.maximum_cylinder
      #...
      x_count += increment_step_size
    end
    x_count = type_of_lens.minimum_cylinder
    y_count -= increment_step_size
  end
end

All this is doing is iterating from some upper bound, maximum_sphere, to a lower bound, minimum_sphere, by some constant increment. The inner loop does the same thing, from a lower bound to an upper bound. Ruby, predictably, has a method for that specific use case, Numeric#step:

1
2
3
4
5
6
7
8
9

STEP_SIZE = 0.25

def populate
  maximum_sphere.step(minimum_sphere, -STEP_SIZE) do |sphere|
    minimum_cylinder.step(maximum_cylinder, STEP_SIZE) do |cylinder|
      #...
    end
  end
end

The Numeric#step method simply calls the associated block once for every increment of the second parameter, from the receiver to the first parameter. In other words, it “steps” (in the case of the outer loop) from maximum_sphere to minimum_sphere in increments of -STEP_SIZE.

Lastly, let’s address the process of creating a new Lens instance. In Dan’s original code, this was done by first creating an empty Lens object, then assigning the relevant attributes one-by-one, and saving:

1
2
3
4
5
6

lens = Lens.new
lens.lens_type_id = type_of_lens.id
lens.sphere = y_count
lens.cylinder = x_count
lens.quantity = 0
lens.save

The Rails way of doing this employs the create method, passing it a hash of the attributes you wish the new object to have. It then returns the newly created (and saved) object. In our version, we ignore the return value, since it is not relevant to what we’re doing—it suffices us to know that the Lens object was created.

1
2

Lens.create :lens_type_id => type_of_lens.id,
  :sphere => y_count, :cylinder => cylinder, :quantity => 0

The final version of the TypeOfLens model:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16

class TypeOfLens < ActiveRecord::Base
  STEP_SIZE = 0.25

  after_create :populate

  # ...
  def populate
    maximum_sphere.step(minimum_sphere, -STEP_SIZE) do |sphere|
      minimum_cylinder.step(maximum_cylinder, STEP_SIZE) do |cylinder|
        Lens.create :lens_type_id => id,
          :sphere => sphere, :cylinder => cylinder, :quantity => 0
      end
    end
  end
  # ...
end

In the end, I think this exercise illustrates nicely the value of learning Ruby, as well as Rails. Certainly, we all must start some place, and Rails offers plenty to learn for newcomers. But once you know the basics, it pays in a big way to keep stretching, learning Ruby idioms for common patterns.

The Rails way, after all, is only a specialized subset of the greater Ruby way.

Comments

Leave a response

  1. anon – December 08, 2006 @ 05:56 AM

    Correction: It’s Numeric#step, not Array#step. Array#step just didn’t make sense to me, so I looked it up.

    I’m still trying to get to where I think in idiomatic Ruby rather than mentally translating perl. Needless to say, this series fits the bill perfectly. Please, keep up the great work.

  2. brett – December 08, 2006 @ 05:58 AM

    I think you have a typo. “Array#step method” should read “Numeric#step method”.

  3. Jamis – December 08, 2006 @ 06:09 AM

    Yup, you’re right, it is Numeric#step. I’ve fixed the article. Thanks for keeping me honest.

  4. dalager – December 08, 2006 @ 07:29 AM

    It’s amazing how good a learning tool this weblog is. Really. Keep up the good work :-)

  5. mathie – December 08, 2006 @ 01:47 PM

    Perhaps a further Rails-like change. It looks to me like TypeOfLens `has_many :lenses` so instead of `Lens.create` how about:

    lenses.create :sphere => sphere, :cylinder => cylinder, :quantity => 0

    and have Rails handle the association automagically?

  6. Paulie – December 08, 2006 @ 01:49 PM

    Cheers, need all the info I can get…

  7. Ben – December 08, 2006 @ 02:56 PM

    Thanks for another great article, a good example of the thin-controller, fat-model pattern. Again showing the beauty of Ruby by reducing a rather large block of code with the populate method into a succinct after_create filter in the model.

    Keep up the good work.

  8. Alain – December 08, 2006 @ 03:02 PM

    I really liked that example because I can more easely relate to the code compared to the previous 4 articles that were a bit harder to understand.

  9. Jacob Fugal – December 08, 2006 @ 03:42 PM

    This probably falls in the realm of advanced, possibly too clever, code, but I’d even abstract away the stepping mechanism from the populate method. Something like this:

    STEP_SIZE = 0.25
    def each_sphere(&blk)
  maximum_sphere.step(minimum_sphere, -STEP_SIZE, &blk)
end
    def each_cylinder(&blk)
  minimum_cylinder.step(maximum_cylinder, STEP_SIZE, &blk)
end
    def populate
  each_sphere do |sphere|
    each_cylinder do |cylinder|
      lenses.create :sphere => sphere, :cylinder => cylinder, :quantity => 0
    end
  end
end

    The benefit isn’t immediately apparent for the case where the looping is only used once (aside from - IMO - a minor increase in readability, which is subjective). But the nature of this loop smells like something that’s probably showing up more than once. This separation abstracts how we get the various sizes and cylinders from when we use them. That way, if we want to change the how later on, it’s in one place and we don’t need to look for all the whens and risk missing one.

    [BTW, I’m sure you’re aware of this Jamis but just didn’t think of it or have space for it. I thought I’d just continue the enlightenment in the comments… :)]

  10. Jim Kane – December 08, 2006 @ 03:54 PM

    Thanks to Jamis for another great critique. I notice that you made no comment on one other idiom that seems to be in need of examination: the if object.save / flash&redirect / else / render pattern (small p). Wouldn’t the Ruby Way involve replacing that if construct with a save! call and a Rescue clause at the end of the method? I’ve started doing this more in my own projects, and it appears a bit cleaner to my eye.

    Or maybe I’m the exception here.

  11. Jamis – December 08, 2006 @ 04:23 PM

    @Jacob, thanks for the addition. You’re right—if loops are being done from min to max more than in that one place, pulling it out into its own iterator is a good idea.

    @Jim, you’re right, that’s a nice pattern. I’ve come to like that one a lot, too, but figured I’d focus most of my energy in this article on idiomatic Ruby, and just touch lightly on the fat model/skinny controller paradigm. Too much meat makes a meal hard to swallow. :)

  12. Entropy – December 09, 2006 @ 12:34 AM

    Please, PLEASE give the model name an endian flip to LensType

  13. Dan Hancu – December 13, 2006 @ 01:54 PM

    Thanks for all the comments and addtional suggestions everyone.

    Entropy, coincidentally, I’ve gone over everything and renamed the model to LensType a few days ago, made things a bit easier.