Blocks and local variable creation

Discussion in 'Ruby' started by John Lane, Apr 15, 2010.

  1. John Lane

    John Lane Guest

    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 ?
    --
    Posted via http://www.ruby-forum.com/.
    John Lane, Apr 15, 2010
    #1
    1. Advertising

  2. John Lane wrote:
    > 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
    --
    Posted via http://www.ruby-forum.com/.
    Brian Candler, Apr 15, 2010
    #2
    1. Advertising

  3. John Lane

    Chris Hulan Guest

    On Apr 15, 6:17 am, John Lane <> wrote:
    > 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 ?
    > --
    > Posted viahttp://www.ruby-forum.com/.


    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
    Chris Hulan, Apr 15, 2010
    #3
  4. John Lane

    Robert Dober Guest

    On Thu, Apr 15, 2010 at 12:17 PM, John Lane <> wrote:
    > 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 }
    > --
    > Posted via http://www.ruby-forum.com/.
    >
    >




    --
    The best way to predict the future is to invent it.
    -- Alan Kay
    Robert Dober, Apr 15, 2010
    #4
  5. John Lane

    John Lane Guest

    Robert Dober wrote:

    > 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...
    --
    Posted via http://www.ruby-forum.com/.
    John Lane, Apr 16, 2010
    #5
  6. On Fri, Apr 16, 2010 at 11:21 AM, John Lane <> wrote:
    > Robert Dober wrote:
    >
    >> 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.
    Jesús Gabriel y Galán, Apr 16, 2010
    #6
  7. On Fri, Apr 16, 2010 at 5:21 AM, John Lane <> wrote:

    > 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
    Rick DeNatale, Apr 16, 2010
    #7
  8. John Lane

    John Lane Guest

    Rick Denatale wrote:
    >
    > 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.

    --
    Posted via http://www.ruby-forum.com/.
    John Lane, Apr 16, 2010
    #8
  9. On Fri, Apr 16, 2010 at 6:09 PM, John Lane <> wrote:
    >
    > 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
    Rick DeNatale, Apr 16, 2010
    #9
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Arjen
    Replies:
    3
    Views:
    439
    Scott Allen
    Feb 27, 2005
  2. matt
    Replies:
    1
    Views:
    259
    George Ogata
    Aug 6, 2004
  3. Thairuby Thairuby
    Replies:
    1
    Views:
    111
    Charles Oliver Nutter
    Jun 8, 2008
  4. Steven Taylor
    Replies:
    9
    Views:
    249
    Brian Candler
    Apr 27, 2009
  5. Kaye Ng
    Replies:
    12
    Views:
    460
    Bala TS
    May 27, 2011
Loading...

Share This Page