From: "David A. Black" <dblack@...>
Date: 2008-10-30T12:47:21+09:00
Subject: Re: Question about case statement and lambdas

Hi --

On Thu, 30 Oct 2008, Jeff wrote:

> I fear this has an obvious answer that I'm just not figuring out.
>
> Sometimes I dive into the Rails source code to see how something works
> (don't worry, this isn't a Rails question).  I have been looking at
> the implementation of the named_scope feature in Rails, and found some
> dense Ruby indeed.  As an exercise, I decided to refactor it to be
> more readable and to force me to understand what it's doing.  A series
> of simple refactorings has led to a change in behavior, and I'm not
> sure why.
>
> Along the way, I ended up with this method (extracted from the
> original implementation):
>
> # Starting point
> def create_lambda_for_scope(name, options, &block)
>  lambda do |parent_scope, *args|
>    Scope.new(parent_scope, case options
>      when Hash
>        options
>      when Proc
>        options.call(*args)
>    end, &block)
>  end
> end
>
> All tests still pass at this point.. but that inline case statement
> bugs me, so I refactor it out to a temporary variable:
>
> # Attempt #2
> def create_lambda_for_scope(name, options, &block)
>  lambda do |parent_scope, *args|
>    arg = case options
>      when Proc
>        options.call(*args)
>      when Hash
>        options
>    end
>    Scope.new(parent_scope, arg, &block)
>  end
> end
>
> Tests still pass, yessss!  But, I still hate seeing that case
> statement.  So I now do this:
>
> # Attempt #3:
>   <snip>
>    arg = (options.is_a?(Proc) ? options.call(*args) : options)
>    Scope.new(parent_scope, arg, &block)
>  <snip>
>
> Test still pass, I'm about to declare victory.  But I can't resist one
> more simplification to remove the temporary. Since the case statement
> is so simple, it looks to me like, if options is a Hash, don't do
> anything, but if it's a Proc, then call it.
>
> So I decide to just get rid of the temporary and simply assign a new
> value to options only if it's a Proc:
>
> # Attempt #4
>  <snip>
>    options = options.call(*args) if options.is_a?(Proc)
>    Scope.new(parent_scope, options, &block)
> <snip>
>
> Kaboom, several tests now fail.
>
> Is there something I don't understand about the difference between #3
> and #4?  I'm guessing it's something to do with the fact that I'm
> building a lambda here, so there's closure scope to be concerned
> about... but still, #3 and #4 look semantically the same to me.
>
> Any insight or ideas would be appreciated.

Expanding on Botp's point about options being changed:

The problem is that if options is a Proc the first time you call the
lambda, it isn't the second time. It's still whatever got assigned to
it.

Say options.call(*args) returns "Hello!". The second time you call the
lambda, options will be "Hello!", and not a Proc. "Hello!" will then
be sent to Scope.new, which is probably not going to be right.

That's why you need a temporary variable or an inlined conditional.
You don't want to trample the options variable itself, since it's the
only place where the original options are stored.


David

-- 
Rails training from David A. Black and Ruby Power and Light:
   Intro to Ruby on Rails  January 12-15   Fort Lauderdale, FL
   Advancing with Rails    January 19-22   Fort Lauderdale, FL *
   * Co-taught with Patrick Ewing!
See http://www.rubypal.com for details and updates!