Question about case statement and lambdas

[Jeff]

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.

Thanks!
Jeff

 

[Peña, Botp]

From: Jeff [mailto:[email protected]]=20
# # Attempt #4
# <snip>
# options =3D options.call(*args) if options.is_a?(Proc)
# Scope.new(parent_scope, options, &block)
# <snip>

the only difference i see is that #4 modifies options

 

[David A. Black]

Hi --

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!

 

[Brian Candler]

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.

And whilst at first glance it seems that 'options' is actually "just" a
local method argument which drops out of scope when the method returns,
it is of course bound into the environment of the lambda (i.e. the
closure), and persists there.

 

[David A. Black]

Hi --

And whilst at first glance it seems that 'options' is actually "just" a
local method argument which drops out of scope when the method returns,
it is of course bound into the environment of the lambda (i.e. the
closure), and persists there.

Yes, I should have made that explicit -- I had the impression the OP
was hip to that part.


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!

 

[Jeff]

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:
=A0 =A0Intro to Ruby on Rails =A0January 12-15 =A0 Fort Lauderdale, FL
=A0 =A0Advancing with Rails =A0 =A0January 19-22 =A0 Fort Lauderdale, FL = *
=A0 =A0* Co-taught with Patrick Ewing!
Seehttp://www.rubypal.comfor details and updates!

Ah! Thanks for that explanation! Makes perfect sense now.

Jeff

purpleworkshops.com

 

[Jeff]

And whilst at first glance it seems that 'options' is actually "just" a
local method argument which drops out of scope when the method returns,
it is of course bound into the environment of the lambda (i.e. the
closure), and persists there.

Excellent... thanks for that insight.

Jeff

purpleworkshops.com