4

Relying on this in the wrong context is a common pitfall. A contrived example:

function SomeClass() {
    this.prop = 42;
    document.body.addEventListener('click', function() {
        alert(this.prop); // will be undefined
    });
}
new SomeClass();

One way around this is using bind (or call or apply), for example:

function SomeClass() {
    this.prop = 42;
    document.body.addEventListener('click', (function() {
        alert(this.prop); // works
    }).bind(this));
}
new SomeClass();

Another option is to make a reference to this available via closure, usually _this or self:

function SomeClass() {
    var _this = this;
    this.prop = 42;
    document.body.addEventListener('click', function() {
        alert(_this.prop); // works
    });
}
new SomeClass();

I prefer the latter method, because I can declare it once rather than wrapping loads of functions with bind(), and nested callbacks (which I know are not ideal, but are seen in the wild) become simpler. But I have the sneaking suspicion that either a) there are gotchas with this method, or b) the fact that I have to do this or bind in the first place means that I'm doing it wrong. Any thoughts?

tobek
  • 4,349
  • 3
  • 32
  • 41

2 Answers2

3

Your method to address the "issue of this" is OK:


function SomeClass() {
    this.prop = 42;
    document.body.addEventListener('click', (function() {
        alert(this.prop); // works
    }).bind(this));
}
new SomeClass();

You really are not doing anything wrong if you found you need doing this, but depending on context, if you are worried about performance maybe you don't want to create new functions (that is what .bind internally does to change this)

Instead of doing the binding, you can use an scope variable, like your second example:


function SomeClass() {
    var _this = this;
    this.prop = 42;
    document.body.addEventListener('click', function() {
        alert(_this.prop); // works
    });
}
new SomeClass();

The only remaining issue here, is about code readability, but I thing is matter of tastes, this what I would do for this particular case


function SomeClass() {
    var prop = 42;
    document.body.addEventListener('click', function() {
        alert(prop); // works
    });
}
new SomeClass();

I just improved readability and maybe a little of performance, but I am not meaning that is "the way" to do it, I am trying to say you should know how to use lexical scoping and closures and use that (or not) where/when you think you have to (all depends on context, of course)

EDIT: wait!, I assigned this.prop = 42 because I need to expose the property On that case, I think the best practice from design PoV is to expose it via a getter:


function SomeClass() {
    var prop = 42;
    document.body.addEventListener('click', function() {
        alert(prop); // works
    });
    this.getProp = function() {
      return prop;
    };

    this.setProp = function(newValue) { // prop is supposed to be modifiable ?
      prop = newValue;
    };
}
new SomeClass();

Another side note about design not related with the "this" issue, it doesn't feel right to interact with global objects from the class, I would do something like thus:


function SomeClass(receiver) {
    var prop = 42;
    receiver.addEventListener('click', function() {
        alert(prop); // works
    });
}
new SomeClass(document.body);
dseminara
  • 11,665
  • 2
  • 20
  • 22
  • Yes, it's know that `bind` is slower due to the "unusual" spec requirements, but typically performance should not be an issue in the places where `bind` is used. – Bergi Sep 17 '14 at 18:06
  • IMHO, readability is enough justification here to avoid using bind – dseminara Sep 17 '14 at 18:09
  • 1
    Uh, many people would argue that *using* `bind` increases readability… Sure, your `var prop` is a nice case-specific improvement, but when `prop` is required to be a property, then both of OP's patterns are fine. – Bergi Sep 17 '14 at 18:18
  • Agree!, it depends on context. I think the general rule is avoiding the antipattern of "use this to hold things", of course this is ok when used for what was designed, but for all other case maybe is better to use a variable with a proper name (my personal opinion) – dseminara Sep 17 '14 at 18:21
2

Are there gotchas with this method?

No, it's fine. I'd even argue that there are less gotchas than with bind (and its ES3 non-support). A pitfall that is sometimes seen is to forget var (and introducing a global variable if not in strict mode), but if you understood closures that's probably not a problem any more.

Or does the fact that I have to do this or bind in the first place mean that I'm doing it wrong.

No, that's just how calling methods that use this works in JavaScript. I'm not saying that a wrong design can't require an abnormally frequent use of self/bind, but good designs do contain this pattern as well.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375