From: lasitha <lasitha.ranatunga@...>
Date: 2009-02-16T22:52:54+09:00
Subject: Re: Hash :: Recursive traversing and stripping the values

On Mon, Feb 16, 2009 at 6:20 PM, lasitha <lasitha.ranatunga@gmail.com> wrote:
> On Mon, Feb 16, 2009 at 5:50 PM, vimal <cool.vimalsmail@gmail.com> wrote:
>>    error suggesting "Error => in `strip_hash_values': stack level too
>> deep (SystemStackError)".
>
>
> Vimal, take a very close look at the recursive call:
>
>> ...
>>        elsif v.class == Hash
>>          strip_hash_values(hash)
>>        end
>> ...
>
> You have a simple typo/bug there.
>

Some further observations:

1.  This line uses a comparison operator, not the assignment operator:
    hash[k] == v.strip!

2.  We're getting away with the above typo because we're using #strip!
(the version with side-effect).  So the string v is being modified in
place and therefore doesn't need to be assigned back to hash[k].
Needless to say this is redundant - we should either use #strip or get
rid of the assignment.

3.  A case statement may be a better choice than if/else.  I tend to
consider a case statement as soon as i have more than two branches -
it reads slightly cleaner and is easier to add new cases to.  See the
example below.

4.  I would consider adding a catch-all clause and failing fast with
an exception if the hash contains an unhandled type.  Even if you
later choose to silently ignore unhandled types, keeping the else
clause in the code usefully documents your intent.  Again, see the
example below.

The code below includes rspec (test) code in case that's of use/inspiration.

Cheers,
lasitha.


The following code can be also be seen at:
http://pastie.org/390656

def strip_hash_values(hash)

  hash.each_value do |v|
    case v
      when String then v.strip!
      when Array  then v.each {|i| i.strip! }
      when Hash   then strip_hash_values(v)
      else raise ArgumentError, "Unhandled type #{v.class}"
    end
  end

end


describe 'strip_hash_values' do

  it 'should recurse and strip hash values' do
    hash =     { c: ' s ', a: [' an ', ' array '], h: { nested: ' hash ' } }
    stripped = { c:  's',  a: [ 'an',   'array' ], h: { nested:  'hash'  } }
    strip_hash_values(hash).should eql(stripped)
  end

  it 'should raise an exception if the hash contains an unhandled type' do
    lambda { strip_hash_values( { e: 1 } ) }.should raise_error(ArgumentError)
  end

end