3
\$\begingroup\$

I need a way to store and retrieve callbacks in Ruby, so I made a Callbacks class. Any suggestions would be greatly appreciated.

class Callbacks
  def initialize
    # name: name of callback
    # event: event of which it belongs
    # callback: block to call
    @callbacks = []
  end

  def add name, event, &block
    if block_given?
      @callbacks << {
        name: name,
        event: event,
        callback: block
      }
    end
  end

  def get options={}
    name = options[:name]
    event = options[:event]

    @callbacks.each do |h|
      if name and event
        return h if h[:name] == name and h[:event] == event
      else
        return h if h[:name] == name or h[:event] == event
      end
    end
    nil
  end
end
\$\endgroup\$
5
  • \$\begingroup\$ Just to be sure, you are returning the callback even if one of the properties does not match. Is this intensional? \$\endgroup\$ Commented May 31, 2012 at 18:16
  • \$\begingroup\$ Yes, it is returning a callback even if one of the options is not defined. \$\endgroup\$ Commented May 31, 2012 at 18:24
  • \$\begingroup\$ In your code, it would match even if both the options are defined but say the name does not match. Surely this is not the correct behavior then? \$\endgroup\$ Commented May 31, 2012 at 18:26
  • \$\begingroup\$ That doesn't sound right. If :name and :event are defined, it searches for a callback that matches both :name and :event. Otherwise, it matches one or the other. \$\endgroup\$ Commented May 31, 2012 at 18:34
  • \$\begingroup\$ Oh ok, my mistake. \$\endgroup\$ Commented May 31, 2012 at 18:43

1 Answer 1

Reset to default
1
\$\begingroup\$

Why not use a hashmap as the main DS? That way, you optimize for when name and event are defined, and suffer only a constant penalty and degrades to linear for other cases. (In your solution the cost is always linear). (As below)

class Callbacks
  def initialize
    @callbacks = {}
  end

  def add(name, event, &block)
    key = {:name => name, :event => event}
    @callbacks[key] = block if block_given?

shouldn't we throw an exception if block is not given?

  end

  def get(o={})

We can use values_at to extract multiple values from a hash.

    name,event = o.values_at(:name,:event)

We don't need to check if name and event separately because equality checking does it for us.

    return @callbacks[o] if name and event
    @callbacks.each do |k,v|
      return v if k[:name] == name or k[:event] == event
    end
    nil
  end
end

And shouldn't you provide a way to unregister from callback?

\$\endgroup\$

You must log in to answer this question.