From: Greg Hauptmann <greg.hauptmann.ruby@...>
Date: 2008-10-28T15:08:50+09:00
Subject: Re: Hash.merge_add! extension - how does this code look?

interesting - is it really Ruby approach to let things 'break' during
a method so to speak as opposed to defensive coding and doing some
form of validation at the beginning of the method?  I noted when I run
my spec's with your code i get the error

     "<NoMethodError: undefined method `keys' for 123:Fixnum>"

which is quite a bit more cryptic than my

      "RuntimeError - "Parameter passed in not a hash"

Wouldn't it be harder to debug code if one were getting the former
error message rather than the later?


On Tue, Oct 28, 2008 at 11:38 AM, Trans <transfire@gmail.com> wrote:
>
>
> On Oct 27, 8:42 pm, "Greg Hauptmann" <greg.hauptmann.r...@gmail.com>
> wrote:
>> thanks all for feedback to date - here's my latest take
>>
>> ------------------------------------
>>   def merge_add!(h)
>>     raise "Parameter passed in not a hash" if !h.instance_of?(Hash)
>
> Again, this is considered poor form. The reason is, if it isn't a Hash
> it will blow up in the next couple of statements anyway, but more
> importantly something other a Hash might emulate one. And there's no
> reason not to allow it to work.
>
>>     # normalise input hash to contain arrays
>>     h.each { |key, value| if !value.instance_of?(Array) then h[key] =
>> [value] end }
>>
>>     self.merge!(h) do |key, existing, new|
>>       existing = existing.instance_of?(Array) ? existing : [existing]
>>       existing + new
>>     end
>>   end
>
>
>  def merge_add!(h)
>    q = {}
>    (keys | h.keys).each do |k|
>      q[k] = Array(self[k]) + Array(h[k])
>    end
>    replace(q)
>  end
>
>
>                              trans.
>
>