3
\$\begingroup\$

I needed a simple and efficient way to use events in my JavaScript code, so I came up with a small event library.

Currently, I'm doing it like this:

// eva.js - The lightweight es6 event library.
var eva = (() => {
  // Prepare local globals
  var eva = {};
  var receivers = [];
  var gid = 0;
  /**
   * Verify the type of a parameter.
   * Throw if the type does not match the expected type.
   * @param {object} item - The argument itself
   * @param {string} name - The name of the argument
   * @param {string} expected - The expected type
   */
  var verifyType = (item, name, expected) => {
    if (typeof (item) !== expected) {
      throw "Argument '" + name + "' has to be of type '" + expected + "'.";
    }
  }
  /**
   * Subscribe to an event.
   * @param {string} name - The name of the event
   * @param {function} action - The event action
   * @returns {number} - The subscription identifier
   */
  var subscribe = (name, action) => {
    verifyType(name, 'name', 'string');
    verifyType(action, 'action', 'function');
    receivers.push({name: name, action: action, id: gid});
    return gid++;
  }
  /**
   * Unsubscribe from an event.
   * @param {number} id - The subscription identifier
   * @returns {boolean} - Whether the subscription has been removed
   */
  var unsubscribe = id => {
    verifyType(id, 'id', 'number');
    var result = false;
    receivers.every((item, index, arr) => {
      if (item['id'] === id) {
        arr.splice(index, 1);
        result = true;
      }
      return !result;
    });
    return result;
  }
  /**
   * Dispatch an event.
   * @param {string} name - The name of the event
   * @param {object} e - The event data
   */
  var dispatch = (name, e) => {
    verifyType(name, 'name', 'string');
    receivers
      .filter(item => name === item['name'])
      .forEach(item => item['action']({event: name, data: e}));
  };
  /**
   * List all receivers.
   * @returns {array} - The receivers
   */
  var list = () => receivers;
  /**
   * Clear all receivers.
   */
  var clear = () => {
    receivers.length = 0;
  };
  // Make functions accessible
  eva.subscribe = subscribe;
  eva.unsubscribe = unsubscribe;
  eva.dispatch = dispatch;
  eva.clear = clear;
  eva.list = list;
  // Return eva object
  return eva;
}());
module.exports = eva;

Since I usually don't write JavaScript code and don't know any best practices and quirks of that language, constructive criticism would be greatly appreciated.

\$\endgroup\$
  • \$\begingroup\$ Question: Doesn't this call all event handlers regardless of what event name you're dispatching? Is that intended? If so, what's the point of subscribing to an event by name? \$\endgroup\$ – Flambino Aug 12 '16 at 23:41
  • \$\begingroup\$ @Flambino That behavior was not intended, thanks for pointing it out. Fixed it \$\endgroup\$ – splitty Aug 13 '16 at 0:06
  • \$\begingroup\$ You would use a name - that makes perfect sense. What I meant was that it didn't make sense to subscribe to a specific name, if the receiver will get called for all other events too. I just didn't know if that was intentional, or if it was a bug \$\endgroup\$ – Flambino Aug 13 '16 at 0:08
  • \$\begingroup\$ @Flambino Yeah I misread your answer. Updated my previous answer to reflect that \$\endgroup\$ – splitty Aug 13 '16 at 0:09
1
\$\begingroup\$

It's not bad, though there are some things I'd do different.

Firstly, I'd use a map of arrays for receivers. Doesn't have to be a Map instance, a simple object literal will do. For instance, subscribe would look like this:

var receivers = {};

var subscribe = (name, callback) => {
  if(typeof callback === 'function') {
    receivers[name] = (receivers[name] || []);
    receivers[name].push(callback);
  } else {
    throw new TypeError("callback must be a function");
  }
}

This makes it simple to dispatch events:

var dispatch = (name, data) => {
  receivers[name].forEach(receiver => receiver({name: name, data: data}));
}

Secondly, I wouldn't use rely on a gid value to track receivers; I'd use the receiver itself. It eliminates needing to track multiple IDs if one function is subscribing to multiple events, and it makes the sub/unsub functions "symmetrical": All you need is an event name and a function.

function someHandler() { ... }

eva.subscribe('someEvent', someHandler);
eva.unsubscribe('someEvent', someHandler);

To achieve this, unsubscribe could look like:

var unsubscribe = (name, callback) => {
  if(receivers[name]) {
    receivers[name] = receivers[name].filter((receiver) => receiver !== callback);
  }
}

Also, one important note: Your list function returns a reference to the receivers array. I don't know if this is intended, but it does mean, that you'd be modify the otherwise-"private" array. For instance, saying eva.list().push("foobar") will cause problems, because it sidesteps all you type-checking, and adds something to the array that your code isn't expecting.

Frankly, I don't know how useful the list function is to begin with, but in your current code, you could halfway fix the issue by simply calling slice() on the receivers array. That'll create a copy, and return that instead:

var list = () => receivers.slice();

However, the contents of the copied array will be references to the same objects that are in "private" receivers array, so you cause mischief:

eva.list()[0].action = "foobar"; // obviously not a proper callback

You could spend the time doing a deep clone of everything, but... eh.

Again, simpler to let the array only contain functions. Even if they're returned as references in a copied array, it's harder to accidentally break things.

If you use an object/map to hold the receivers as I suggest above, you do have to do some more complex cloning to return it safely (although the arrays it contains can be copied safely with slice). However, I'd just scope everything in the API to a name, including list and clear:

var list = (name) => receivers[name] ? receivers[name].slice() : [];

var clear = (name) => receivers[name] = [];

If you want to could add a method to return all registered event names by using Object.keys(receivers).

You'll note that I haven't bothered much with type-checking. Only when subscribing do I check that the callback is actually a function. But name arguments? Don't really care.

Type-checking isn't super necessary in JS. Often it really doesn't matter due to type coercion, or you explicitly cast/parse if you need to. Other times it's just simpler to say "garbage in, garbage out": If someone really wants to use a float as an event name, let them. And let them sort it out, if things don't work. This is what I usually lean toward. Since JS isn't strictly typed (and sometimes plain weird), the effort spent implementing your own type-checking could be spent better elsewhere.

All together, minus comments:

var eva = (() => {
  var receivers = {};

  var subscribe = (name, callback) => {
    if(typeof callback === 'function') {
      receivers[name] = (receivers[name] || []);
      receivers[name].push(callback);
    } else {
      throw new TypeError("callback must be a function");
    }
  }

  var unsubscribe = (name, callback) => {
    if(receivers[name]) {
      receivers[name] = receivers[name].filter((receiver) => receiver !== callback);
    }
  }

  var dispatch = (name, data) => {
    receivers[name].forEach(receiver => receiver({name: name, data: data}));
  }

  var list = (name) => receivers[name] ? receivers[name].slice() : [];

  var clear = (name) => receivers[name] = [];

  return {
    subscribe: subscribe,
    unsubscribe: unsubscribe,
    dispatch: dispatch,
    list: list,
    clear: clear
  };
}());
\$\endgroup\$
  • 1
    \$\begingroup\$ As always quite impressive \$\endgroup\$ – konijn Aug 15 '16 at 13:19
  • \$\begingroup\$ @konijn Thank you - that's dangerously high praise :) \$\endgroup\$ – Flambino Aug 15 '16 at 13:59

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.