3
\$\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

// ...
\$\endgroup\$
2
\$\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.

\$\endgroup\$
2
\$\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.

\$\endgroup\$
  • \$\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\$ – Moritz Friedrich Nov 16 '15 at 23:05

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.