Use a wrapping class for Events

[faultyserver]

Right now, when an event gets sent up the middleware stack, the call method takes a specific set and arrangement of arguments:

def call event, channel, *args, **kwargs

This could be simplified and made more powerful by wrapping everything up into a Shark::Event class. With that, the call definition would be much simpler:

def call event

Additionally, the semantics of argument collectors (*, and **) can be avoided, as everything will be wrapped up into args and kwargs attributes on Event.

A basic outline of the class would be

class Shark::Event
  # The topic this event is being fired on
  attr_accessor :topic
  # The type of event being represented
  attr_accessor :type
  # The positional arguments for this event
  attr_accessor :args
  # The keyword arguments for this event
  attr_accessor :kwargs
  # The object that caused the creation of this event
  attr_accessor :originator
end

Another advantage of using a wrapping class is that arguments can be freely modified out-of-place. Currently, modifying arguments either has to be done in-place (which only works in some cases, e.g. args.map!{}), or a replacement has to be specified when proxying an event up the middleware stack. This gets troublesome with event handlers, because there is no way to replace an argument (using = instead of bang methods) and have that change persist outside of the handler. For example:

class NonEventMiddleware < Shark::Middleware
  # The block given here is called for every event passing through this middleware
  # that doesn't have a specific handler registered.
  default_event_handler do |channel, args, kwargs|
    # This works, because `args` is being modified in-place. The change will persist
    # through to the next middleware.
    args.map!{ |arg| arg.to_json }
    # This doesn't work, because the assignment simply changes the local binding
    # of `kwargs`, not the object it represents.
    kwargs = { foo: :bar, originator: 'me' }
  end
end

With an Event class, this would look and work differently:

class EventMiddleware < Shark::Middleware
  default_event_handler do |event|
    # Modification in-place still works, but is unnecessary.
    event.args.map!{ |arg| arg.to_json }
    # This would also work instead
    event.args = event.args.map{ |arg| arg.to_json }
    # This would also work, now, since `event.kwargs` (an object reference) is being
    # modified, not `event` (the local binding).
    event.kwargs = { foo: :bar, originator: 'me' }
  end
end

The only thing that doesn't work here, is replacing the entire event object, which would be undesirable anyway.

[elliottwilliams]

Semantics 👍

Does this make any functionality easier for you to implement? It looks like a pretty simple change, but if it's going to require nontrivial refactors, I would keep this low priority until it blocks something else.

[faultyserver]

The example I gave is exactly why I brought this up.

In Serializer, I need to map arguments to their serialized form. The most efficient/dry way to implement this is as a default_event_handler, which doesn't allow me to do out-of-place assignment to args (as shown in the example). But, with the Event wrapper, it would.

The refactors are all pretty trivial: Find/replace args and kwargs in middleware classes with event.args and event.kwargs, and change all of the function/block definitions to take event rather than channel, args, kwargs.

[elliottwilliams]

I see. Thanks for clarifying!

[faultyserver]

Closed by #18.