Is it wise to do event binding in the prototype & why is the context of this lost?

Question

When creating modular JavaScript code for web pages, I often need to attach events to e.g. buttons.

Take the following example code, typically found in an AMD module:

define(function(require) {

    var MyObject = function(ele) {
        this.ele = ele;
    }

    MyObject.prototype = {
        myfunct: function() {
            console.log('myfunct called');
        }
    }

    return MyObject;

});

Later on the page I would do:

$(document).ready(function() {
    var button = $('#button'),
        myobj = new MyObject(button);

    button.click(function() {
        myobj.myfunct();
    });
});

This works, but still seems to be a bit unclean I think.

For example, I need to create at least one variable in the global namespace to bind a function to a button. Also, when there are many JavaScript powered interactions on a page, the code gets messy – which is something I initially wanted to tackle by using modular JavaScript.

Thats why my idea was to to the event binding inside the prototype:

var MyObject = function(ele) {
    self = this;
    this.element = ele; 

    this.init();
}

MyObject.prototype = {
    init: function() {        
        $(this.element).click(function() {
           self.myfunct();
        });
    },
    myfunct: function() {
        console.log('myfunct called');
    }
}

That way, the code outside of the module would look like this:

$(document).ready(function() {
    var button = $('#button'),
        myobj = new MyObject(button);

});

Is it wise to move the event binding into the prototype? If so, is it okay the way I have done it, or is there a way using init()?

In addition, I've noticed that when there are two buttons on a page, some context is lost – self always refer to the last instance of MyObj.

Why is this happening – I thought with using self = this; I could prevent the context?

Fiddle

Answer 1

Ok, first what's happening with self.

With this line:

self = this;

you are creating a global variable called self that gets overwritten every time your constructor is called. This could have been easily detected if you were using strict mode. Also, if you were using a local variable correctly, your prototype would have no idea what self, so you attempt to use self in the prototype is broken.

I think there are problems with both of your approaches:

• The first approach requires too much manual work outside of your MyObject type.
• The second approach (if it worked correctly) attaches events to the button as a side effect of calling the constructor. This is confusing to someone using your API because one expects a constructor to create an object, not to modify other existing objects.

As a remedy, I would propose the following approach:

var MyObject = function(ele) {
    this.element = ele; 
}

MyObject.prototype = {
    attachEvents: function() {        
        var self = this;
        $(this.element).click(function() {
           self.myfunct();
        });
    },
    myfunct: function() {
        console.log('myfunct called');
    }
};

$(document).ready(function() {
    var button = $('#button'),
        myobj = new MyObject(button);

    myobj.attachEvents();
});

This requires one extra step on the part of the person instantiating the MyObject, but it clearly conveys the intent of attaching events to myobj's encapsulated elements. It also doesn't require someone using a MyObject to do the intricate maneuvering of your first approach.

Answer 2

There's nothing wrong with doing the event binding the way you are. The reason you're losing the scope is because you did

self = this;

which created self as a global variable, in the constructor function. So every time you call the constructor it sets self to that instance.

To fix it, set self as a local variable in your init function:

MyObject.prototype = {
  init: function() {
    var self = this;  // <-- this is the fix      
    $(this.element).click(function() {
       self.myfunct();
    });
} 

Answer 3

Let's start from the second question. The problem with your code is that you declare self as global variable because you forgot/omitted var keyword. As the result when you create two or more instances the last one overwrites previous and self inside all click events points to the last instance.

Check the fixed code. Note that you have to move var self = this to init method, because now it's local variable:

var MyObject = function(ele) {
    this.element = ele; 
    this.init();
}

MyObject.prototype = {
    init: function() {        
        var self = this;
        $(this.element).click(function() {
           self.myfunct();
        });
    },
    myfunct: function() {
        console.log('myfunct called');
    }
}

As for the first question, it's alright it's your design and there is nothing wrong with it. Binding events in init method is indeed cleaner.