3
\$\begingroup\$

The main purpose of Ruby is to be readable. I hope I did a good job with this gem I made. If there's any kind of suggestion of how to make this better, then please tell me.

class Trigger
  def initialize event, *callbacks
    @callbacks = callbacks
    @event = event

    if @callbacks[0].is_a? TrueClass
      @progression = true
      @callbacks.delete_at(0)
    elsif @callbacks[0].is_a? FalseClass
      @progression = false
      @callbacks.delete_at(0)
    else
      @progression = false
    end
  end

  def trigger(*args)
    case @event
    when Proc
      event_data = @event.call
    when Method
      event_data = @event.call
    else
      event_data = self.method(@event).call(*args)
    end
    @callbacks.each do |callback|
      if callback.instance_of? Trigger
        if @progression
          callback.trigger(*args, event_data)
        else
          callback.trigger(*args)
        end
      else
        case callback
        when Proc
          if @progression
            callback.call(*args, event_data)
          else
            callback.call(*args)
          end
        when Method
          if @progression
            callback.call(*args, event_data)
          else
            callback.call(*args)
          end
        else
          if @progression
            method(callback).call(*args, event_data)
          else
            method(callback).call(*args)
          end
        end
      end
    end
  end

  #triggers the callbacks without executing the original method
  def silent_trigger(*args)
    @callbacks.each do |callback|
      if callback.instance_of? Trigger
        callback.trigger(*args)
      else
        case callback
        when Proc
          callback.call
        when Method
          callback.call
        else
          method(callback).call(*args)
        end
      end
    end
  end

  # add callback(s) to instance
  def add(*callbacks)
    @callbacks.concat callbacks
  end

  def insert(index, *callbacks)
    @callbacks.insert(index, callbacks)
  end

  # remove callback(s) from instance
  def remove(*callbacks)
    callbacks.each do |callback|
      @callbacks.delete_at(@callbacks.index(callback) || @callbacks.length)
    end
  end

  def delete_at(index)
    @callbacks.delete_at(index)
  end

  def remove_all
    @callbacks = []
  end

  # fetch info from instance
  def index(callback)
    @callbacks.index(callback)
  end

  def event_name
    @event
  end

  def list
    @callbacks
  end
end

And if you want to try it out, here's a quick little console program to show you how it works:

$foobar = 0

def foo
  $foobar += 1
end

def bar
  puts $foobar
end

# create Trigger and callback, then trigger the Trigger
_foo = Trigger.new(:foo, :bar)
_foo.trigger

# create a method to determine if bar has been called before
$bar_called? = false
def bar_called
  $bar_called = true
end

# create a callback for the callback
_bar = Trigger.new(:bar, :bar_called)

# replace old callback with new one (note: you can use Methods, Procs, Symbols, Strings, or other Triggers too)
_foo.remove(:bar)
_foo.add(_bar)

# methods are not called when added to Trigger callbacks, only when triggered
puts $bar_called?
_foo.trigger
puts $bar_called?

You can find the gem here.

\$\endgroup\$
  • \$\begingroup\$ added an answer, but now that i check your console example, I think you have a mistake on your logic : when you pass in a symbol as a callback, the method will be called on the Trigger instance, not on the caller's context as your console code suggests. It's better to use a proc or block in this case. \$\endgroup\$ – m_x Oct 20 '13 at 12:51
  • \$\begingroup\$ Andrew, have a close look at @m_x's answer. I think it's very nice, preferable to mine. If you agree, feel free to change your preferred answer selection. \$\endgroup\$ – Cary Swoveland Oct 20 '13 at 18:06
4
\$\begingroup\$

1) get rid of conditionals

I think your main problem here is the nested conditionals that litter your code. This increases complexity and tends to be less readable.

You can get rid of those conditionals using a method like this :

def make_callable(object)
  case object
  when Proc, Method then object
  when Trigger      then ->(*args){ object.trigger(*args) }
  else                   ->(*args){ public_send object, *args }
  end
end

so you can do things like :

@event = make_callable(event)
@callbacks = callbacks.map{ |c| make_callable c }

This way, all your callbacks will respond to call uniformly, so you won't need conditionals anymore.

2) use inheritance

As i see it, your @progression instance variable masks the need for two different behaviors, which means two different classes : a "silent" trigger, and a "verbose" one that extends the former.

class Trigger

  # factory method to instantiate the right type of callback.
  # I slightly changed the signature from the original #initialize
  # as I thought it would make more sense this way, 
  # but it is possible to keep the original one with minor tweaks
  #
  def self.factory(verbose, event, *callbacks)
    verbose ? Verbose.new(event, *callbacks) : new(event, *callbacks)
  end

  def initialize(event, *callbacks)
    @event = make_callable(event)
    @callbacks = callbacks.map{ |c| make_callable c }
  end

  # SNIP : this class would also expose add_callback, remove_callback, etc.

  def trigger(*args)
    @callbacks.each{ |c| c.call(*args) }
  end

  private 

  def make_callable(object)
    case object
    when Proc, Method then object
    when Trigger      then ->(*args){ object.trigger(*args) }
    else                   ->(*args){ public_send object, *args }
    end
  end
end

class Trigger::Verbose < Trigger
  def trigger(*args)
    event_data = @event.call(*args)
    super(*args, event_data)
  end
end

As you can see, this simplifies the logic a lot, and makes clear that we have two different behaviors, which is invaluable for consumers of your API.

\$\endgroup\$
  • \$\begingroup\$ About the super in the Verbose class, doesn't it need to have the name of the method to invoke? Or does it by default invoke the same method in it's superclass? \$\endgroup\$ – Andrew Oct 23 '13 at 20:44
  • 1
    \$\begingroup\$ super just tells ruby to invoke the same method up the inheritance chain. if you use super alone, it will call the parent class' method with the same arguments as the caller, that's why i pass in the extra argument, and that's why you sometimes see super() (to force calling without arguments) \$\endgroup\$ – m_x Oct 23 '13 at 20:49
  • \$\begingroup\$ And by the last statement "to force calling without arguments" you mean the arguments being which methods to call, right? \$\endgroup\$ – Andrew Oct 23 '13 at 20:56
  • \$\begingroup\$ no. super is a bit peculiar, it's not a method, it's a reserved word (a built-in feature of the language). Think about superas a sort of placeholder for "the same method in the _super_class". See this (section "inheritance and messages") for more in-depth information. \$\endgroup\$ – m_x Oct 23 '13 at 21:04
  • \$\begingroup\$ Oh, I misread what you were intending to saying in the other post. Also, to further explain, @progression doesn't differentiate between a "silent" trigger and a "verbose" trigger. It defines whether or not to pass the return value of the event to its callbacks. The reason why I didn't really explain this in the question is because I wanted to see if people could understand what it does from the code itself. \$\endgroup\$ – Andrew Oct 23 '13 at 21:15
2
\$\begingroup\$

Here a some suggestions for the first three methods:

  def initialize event, *callbacks
    @callbacks = callbacks
    @event = event

    case @callbacks[0]
    when TrueClass
      @progression = true
      @callbacks.delete_at(0)
    when FalseClass
      @progression = false
      @callbacks.delete_at(0)
    else
      @progression = false
    end
  end

  def trigger(*args)
    arguments = args.dup
    if @progression
      arguments << case @event
      when Proc, Method
        @event.call
      else
        method(@event).call(*args)
      end
    end
    silent_trigger(*arguments)
  end

  #triggers the callbacks without executing the original method
  def silent_trigger(*args)
    @callbacks.each do |callback|
      case callback
      when Trigger
        callback.trigger(*args)
      when Proc, Method
        callback.call
      else
        method(callback).call(*args)
      end
    end
  end

I didn't check the code carefully, so there could be a few minor problems to fix.

\$\endgroup\$
  • \$\begingroup\$ I see a problem in the if @progression right there. It won't trigger the event if you have @progression set to false. I'm also guessing that @progression isn't exactly self-explanatory without knowing what it does (seeing as I didn't use an example of it in my original question), but also after reviewing what you wrote, I found some typos in my own script. Thanks for spending some of your time on me! \$\endgroup\$ – Andrew Oct 10 '13 at 16:54
  • 1
    \$\begingroup\$ When you're finished with mods, consider editing your question to include the GitHub address. \$\endgroup\$ – Cary Swoveland Oct 10 '13 at 21:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.