Re: Observer patch

On Fri, 8 Jul 2005, nornagon wrote:

> Hi all.
         [...]
> I request the inclusion of this patch in Ruby 1.8. It is a trivial
> patch, and I myself cannot find any bugs -- though if someone else
> would be kind enough to give it a quick going-over, I'd be much
> obliged.
>
> Without further ado, I present The Patch.
>
> --- observer.rb 2005-07-05 09:28:14.452114352 +1000
> +++ observer-new.rb     2005-07-05 09:36:49.987741024 +1000
> @@ -120,12 +120,12 @@
>  # Add +observer+ as an observer on this object. +observer+ will now receive
>  # notifications.

You've changed how this is called.  IMHO you should patch the
leading RDOC comment.

>  #
> -  def add_observer(observer)
> -    @observer_peers = [] unless defined? @observer_peers
> -    unless observer.respond_to? :update
> -      raise NoMethodError, "observer needs to respond to `update'"
> +  def add_observer(observer, func=:update)
> +    @observer_peers = {} unless defined? @observer_peers

is

        @observer_peers ||= {}

more idiomatic?   Performance??

> +    unless observer.respond_to? func
> +      raise NoMethodError, "observer does not respond to `#{func.to_s}'"
>    end
> -    @observer_peers.push observer
> +    @observer_peers[observer] = func
>  end
>
>  #
> @@ -181,9 +181,9 @@
>  def notify_observers(*arg)
>    if defined? @observer_state and @observer_state
>      if defined? @observer_peers
> -       for i in @observer_peers.dup
> -         i.update(*arg)
> -       end
> +       @observer_peers.each { |k, v|
> +      k.send v, *arg
> +    }

if nitpick.allowed? -- aren't we moving towards having {} for
one-liners and do...end for multi-line blocks?  Giving 2 possible
patches for this patch :-)

>      end
>      @observer_state = false
>    end
>
> --
> - nornagon
>

         HTH
         Hugh