Blocks and local variable creation

[John Lane]

Hello,

I have a simple method:

def rights_for_item (rights_hash, item)
rights_hash.each { |k,v| (rights ||= []) << k if v.include? item }
rights
end

This does not work because "rights" is created local to the block on
"rights_hash.each" instead of local to the method itself. This makes the
statement returning the value of "rights" to fail because there is no
method local variable called "rights".

I can do this instead

def rights_for_item (rights_hash, item)
rights = Array.new
rights_hash.each { |k,v| rights << k if v.include? item }
rights
end

The problem is this returns an empty array rather than nil if no matches
are found. So I do this:

def rights_for_item (rights_hash, item)
rights = Array.new
rights_hash.each { |k,v| rights << k if v.include? item }
rights.empty? ? rights : nil
end

But I don't think it is good idiomatic ruby code. Is there a better way
to write this type of thing ?

 

[Brian Candler]

I have a simple method:

def rights_for_item (rights_hash, item)
rights_hash.each { |k,v| (rights ||= []) << k if v.include? item }
rights
end

This does not work because "rights" is created local to the block on
"rights_hash.each" instead of local to the method itself.

Just prepare it outside the block first:

def rights_for_item(...)
rights = nil
rights_hash.each { ... }
rights
end

But I don't think it is good idiomatic ruby code. Is there a better way
to write this type of thing ?

It's arguably more idiomatic to return an empty array than to return
nil. It's more consistent from the user's point of view. e.g. they can
do

if rights_for_item(x).include? "update"
.. do something
end

If there is a possibility of returning nil then it is more awkward:

r = rights_for_item(x)
if r && r.include? "update"
.. do something
end

 

[Chris Hulan]

Hello,

I have a simple method:

def rights_for_item (rights_hash, item)
   rights_hash.each { |k,v| (rights ||= []) << k if v.include? item}
   rights
end

This does not work because "rights" is created local to the block on
"rights_hash.each" instead of local to the method itself. This makes the
statement returning the value of "rights" to fail because there is no
method local variable called "rights".

I can do this instead

def rights_for_item (rights_hash, item)
   rights = Array.new
   rights_hash.each { |k,v| rights << k if v.include? item }
   rights
end

The problem is this returns an empty array rather than nil if no matches
are found. So I do this:

def rights_for_item (rights_hash, item)
   rights = Array.new
   rights_hash.each { |k,v| rights << k if v.include? item }
   rights.empty? ? rights : nil
end

But I don't think it is good idiomatic ruby code. Is there a better way
to write this type of thing ?

Hash has a select method, in 1.8 (at least according to the docs) it
will a return an array of key,value pairs, in 1.9 it creates a hash
with the selected key,value pairs
You can use the keys method to get the key values as an array

Rather than have the method return two different types (array or nil)
why not accept an empty array as the valid indication of no rights?

Cheers

 

[Robert Dober]

Hello,

But I don't think it is good idiomatic ruby code. Is there a better way
to write this type of thing ?

I do not reallly understand why you want nil, instead of [], but apart
of this I would do
select{|_,v| v.include? item}.map(&:first)

or if you insist

inject( nil ){ |r,(k,v)| v.include?( item ) ? (r||[]) << k : r }

 

[John Lane]

I do not reallly understand why you want nil, instead of [], but apart
of this I would do
select{|_,v| v.include? item}.map(&:first)

or if you insist

inject( nil ){ |r,(k,v)| v.include?( item ) ? (r||[]) << k : r }

Can you please explain the line:

select{|_,v| v.include? item}.map(&:first)

I've never seen the _ before where I would expect a variable. I also
don't understand (&:first) on the call to map.

The reason I was returning nil was so I can the write code like this:

rights_held = rights_for_item (rights_hash, item)
perform_something_with_rights unless rights_hash.nil?

perhaps I should change that thought process to this:

rights_held = rights_for_item (rights_hash, item)
perform_something_with_rights unless rights_hash.empty?

I had thought use of nil was the right way but I'm learning so I'm happy
to be convinced otherwise...

 

[Jesús Gabriel y Galán]

I do not reallly understand why you want nil, instead of [], but apart
of this I would do
select{|_,v| v.include? item}.map(&:first)

or if you insist

inject( nil ){ |r,(k,v)| =A0v.include?( item ) ? (r||[]) << k : r }

Can you please explain the line:

select{|_,v| v.include? item}.map(&:first)

I've never seen the _ before where I would expect a variable.

It's a regular variable name. A block can receive several parameters:

select {|k,v| v.include? item}

It's a convention that indicates that you are not interested in using
that variable.

I also
don't understand (&:first) on the call to map.

Search google for Symbol#to_proc, there are many explanations out
there that are better than what I would be able to explain (in few
words: Ruby tries to convert and object to a proc when you use it like
this, calling the to_proc method).

The reason I was returning nil was so I can the write code like this:

rights_held =3D rights_for_item (rights_hash, item)
perform_something_with_rights unless rights_hash.nil?

perhaps I should change that thought process to this:

rights_held =3D rights_for_item (rights_hash, item)
perform_something_with_rights unless rights_hash.empty?

I had thought use of nil was the right way but I'm learning so I'm happy
to be convinced otherwise...

Others have already talked about this, and I agree it's better to
return the same type of object from a method, and nil and an array are
different types. Don't know how to convince you .

Jesus.

 

[Rick DeNatale]

The reason I was returning nil was so I can the write code like this:

rights_held = rights_for_item (rights_hash, item)
perform_something_with_rights unless rights_hash.nil?

I think you meant rights_held instead of rights_hash in that second line.

perhaps I should change that thought process to this:

rights_held = rights_for_item (rights_hash, item)
perform_something_with_rights unless rights_hash.empty?

My inclination would be to do something like

perform_something_with_rights(rights_for_item(rights_hash, item))

Let the thing which needs the rights deal with what rights it has, and
an empty collection(array) of rights seems a better representation of
empty rights than nil.


--
Rick DeNatale

Blog: http://talklikeaduck.denhaven2.com/
Github: http://github.com/rubyredrick
Twitter: @RickDeNatale
WWR: http://www.workingwithrails.com/person/9021-rick-denatale
LinkedIn: http://www.linkedin.com/in/rickdenatale

 

[John Lane]

I think you meant rights_held instead of rights_hash in that second
line.

Yes I did' Serves me right for doing two things at once

My inclination would be to do something like

perform_something_with_rights(rights_for_item(rights_hash, item))

Let the thing which needs the rights deal with what rights it has, and
an empty collection(array) of rights seems a better representation of
empty rights than nil.

Yes, I agree. I'm convinced of the argument for returning the same type.
Makes a lot of sense when you come to think of it.

I found the explanation from Jesus about (&:first) very interesting but
I'm not sure I think it improves readability. I read these:
http://pragdave.pragprog.com/pragdave/2005/11/symbolto_proc.html
http://www.ruby-forum.com/topic/161089

Personally I think the Symbol#to_proc approach is less idomatic than the
long hand version but I guess it's a matter of personal taste.

 

[Rick DeNatale]

I found the explanation from Jesus about (&:first) very interesting but
I'm not sure I think it improves readability. I read these:
http://pragdave.pragprog.com/pragdave/2005/11/symbolto_proc.html
http://www.ruby-forum.com/topic/161089

Personally I think the Symbol#to_proc approach is less idomatic than the
long hand version but I guess it's a matter of personal taste.

Actually, given the definition of idiom, I'd argue that it's MORE idiomatic.

The Symbol#to_proc idea has been around ruby for quite a while. It
got picked up in Rails some time ago, and is popular in the rails
community despite a certain cost in performance which I believe has
been ameliorated in Ruby 1.9 which made it part of the core.


--
Rick DeNatale

Blog: http://talklikeaduck.denhaven2.com/
Github: http://github.com/rubyredrick
Twitter: @RickDeNatale
WWR: http://www.workingwithrails.com/person/9021-rick-denatale
LinkedIn: http://www.linkedin.com/in/rickdenatale