4
\$\begingroup\$

In my web projects I often need a simple way to trigger callbacks defined somewhere else. Leaving jQuery finally behind me, I was missing their simple event system and tried to create my own. I'd like to know if this is a good implementation since I am not that proficient with Javascript yet.

function Events ()
{
 /**
 * handlers for an event
 *
 * @var {object}
 */
 this.stack = {}
 /**
 * on function.
 * adds a callback to an event stack
 * 
 * @param {string} event the event to subscribe to
 * @param {function} callback the event callback to subscribe
 *
 * @return void
 */
 this.on = function (event, callback)
 {
 // if there is no callback registered for this event yet,
 // create an array for this event
 if (! this.stack.hasOwnProperty(event)) {
 this.stack[event] = []
 }
 // push the event callback to the handler stack
 this.stack[event].push(callback)
 }
 /**
 * off function.
 * removes a callback from an event stack
 * 
 * @param {string} event the event to unsubscribe from
 * @param {function} callback the event callback to unsubscribe
 *
 * @return {bool} whether the callback was removed or not
 */
 this.off = function (event, callback)
 {
 // if the event does not exist, return false
 if (! event in this.stack) return false
 // iterate over handlers, remove the callback in question
 this.stack[event] = this.stack[event].filter(
 function(item) {
 if (item !== callback) {
 return item
 }
 }
 )
 return true
 }
 /**
 * trigger function.
 * calls all registered callbacks for an event
 * 
 * @param {string} eventName the event to trigger callbacks for
 * @param {mixed} data optional data to hand over to the callbacks
 * 
 * @return {bool} whether the event has been fired
 */
 this.trigger = function (event, data)
 {
 // use an empty object if no data given
 var data = (typeof data === 'undefined') ? {} : data;
 // the event does not exist.
 if (! this.stack.hasOwnProperty(event)) return false
 // iterate over all callbacks, run them as expected
 for (var i = 0; i < this.stack[event].length; i++) {
 // @TODO: the scope should maybe be modifyable, even though
 // this would require to specify a scope parameter
 // for every event trigger...
 this.stack[event][i].call(window, event, data)
 }
 return true
 }
}

The Events class can later be instantiated and used in other objects like so:

// ...
this.eventHandler = new Events
this.eventHandler.on('testEvent', function(event, data) {
 console.log(event + ' fired! Data: ' + data)
})
this.eventHandler.trigger('testEvent', 'test data') // testEvent fired! Data: test data
// ...
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Nov 16, 2015 at 10:06
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

Overall it looks pretty good. There are a couple of improvements.

  • Stylistically, JavaScript doesn't place the { character on a new line. You'll see it start on the same line separated by one space:

    function Events() {
     // ...
    }
    if (condition) {
     // ...
    } else if (condition) {
     // ...
    } else {
     // ...
    }
    
  • Since you are not using any function closure variables, there is no need to recreate the on and trigger methods each time the Events constructor function is called. Use the prototype property instead:

    function Events() {
     this.stack = [];
    }
    Events.prototype = {
     stack: null,
     constructor: Event,
     on: function (event, callback) {
     // ...
     },
     off: function (event, callback) {
     // ...
     },
     trigger: function (event, data) {
     // ...
     }
    };
    
  • You could remove the need for the new operator with a little constructor function trickery:

    function Events() {
     if (!(this instanceof Events)) {
     return new Events();
     }
     this.stack = [];
    }
    

    This would allow you to generate a new Events object by calling Events():

    Events().on("foo", function(event) {
     // ...
    });
    
  • The word Events makes this seem like a collection of existing events. Really you've created an event dispatcher, and EventDispatcher feels like a better name.

answered Nov 16, 2015 at 14:46
\$\endgroup\$
3
\$\begingroup\$

I object to your use of stack. In what sense is it a "stack"? It doesn't maintain stack discipline (you can remove any callback using off(), not just the most recently added one), nor do you access them in a last-in, first-out order in trigger().

In other words, it's just an associative array of lists. Naming it stack is misleading and confusing. I suggest naming it callbacks.

answered Nov 16, 2015 at 19:07
\$\endgroup\$
1
  • \$\begingroup\$ You are absolutely correct. As English is not my mother tongue, sometimes it's difficult to name variables the right way. I used stack as the way callbacks are being run is one after another, as they have been added to the stack. Besides, there are no easy ways to move callbacks in the stack. \$\endgroup\$ Commented Nov 16, 2015 at 23:05

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.