Skip to main content
1 of 6

looks good to me, but your questions are hard to evaluate with such a small snippet.

(1) Efficiency

Basically, what else can you do? You have to iterate over all registered events.

(2) Organisation

Seems like a good idea to me. This way you would have central point for binding events to DOM elements - if i'm understanding your code correctly.

(3) OOP

Go for it!

(4) Context

This is problematic, but not dramatic! Basically your .off function does not keep the to arrays in sync. - which is really hard, because events are asynchronous. Consider this example with simple inheritance:

<html>
<body>
<div id="clickMe">Click Me!</div>
<script>
(function(){

    var Hub = Object.create ({});
    //initialize this hub
    Hub.init = function () {
     this.events = {};
     this.contexts = {};
     return this;
    };
    //register a function and context
    Hub.on = function (name, callback, contexts) {
     // create event type if it isn't already and push our callback
     (this.events [name] || (this.events [name] = [])).push (callback);
     (this.contexts [name] || (this.contexts [name] = [])).push (contexts);

     
    };
    //un-register a function
    Hub.off = function (name, callback) {
     // if this event type exists, splice out our callback
     console.log(this.events [name].indexOf (callback));
     console.log((this.events));
     console.log(JSON.stringify(this.contexts));
     this.events [name] && this.events [name].splice (this.events [name].indexOf (callback), 1);
     this.contexts [name] && this.contexts [name].splice (this.events [name].indexOf (callback), 1);
     console.log((this.events));
     console.log(JSON.stringify(this.contexts));
    };
    //fire all of a type of functions
    Hub.trigger = function (name) {
          
     if (!this.events [name] || this.events [name].length === 0) return;
     var args = Array.prototype.slice.call (arguments, 1),
         i = 0, event = this.events [name], context = this.contexts [name], l = event.length;
     // if this event type exists, run all the callbacks
     for (; i < l; event [i].apply (context [i++], args));


    };


    var Main = {};

    Main.hub = Object.create(Hub);
    Main.hub.init();


    var Foo = {};
    Foo.init = function (name) {

        this.name = name;
        Main.hub.on("lala", this.onClick, this);
    
    };
    Foo.onClick = function Foo_onClick (){

        alert(this.name);
    
    };

    var Bar = Object.create(Foo);;
    Bar.init = function (name) {

        Foo.init.call(this, name);
    
    
    };
    Bar.onClick = function Bar_onClick (){

        Foo.onClick.call(this);
        Main.hub.off("lala", this.onClick);

    };

    var f = Object.create(Foo);
    var b = Object.create(Bar);
    var c = Object.create(Foo);

    f.init("foo");
    b.init("bar");
    c.init("c");

    console.log(Object.is(f,c));

    var clickMe = document.getElementById("clickMe");
    clickMe.addEventListener("click",function(e){

        Main.hub.trigger("lala",e);
    
    });


}());
</script>
</body>
</html>

Here, calling Hub.off inside an event callback forces Hub.events.length to be reduced to 2 while i in Hub.trigger is still 3, so event[i] will is undefined.

(5) Messes

Besides (4) i'd recommend another formating style, yours is really hard to read. This is important, if you consider your product to be maintained in the long run.