Wrong
Having to create a variable outside the nested scope so that when you refer to this you refer to the parent's scope.
var Increaser = function(amount) {
this.amount = amount;
};
Increaser.prototype.add = function(value) {
if (Array.isArray(value)) {
var that = this; // NOTE!
return value.map(function(item) {
return item + that.amount;
});
} else {
return value + this.amount;
}
};
var inc = new Increaser(2);
console.log(inc.add(10)); // 12
console.log(inc.add([1, 2, 3])); // [3, 4, 5]
Why it's bad. Because it's code smell. Meaning, it's a hack that goes against what's natural. Also, because it's not necessary. There is a better solution. Hold tight. Code smells have a tendency to get worse. In this case we only have 1 function with its own scope so we can allow ourselves to call it just "that". If it was more complex, we'd have to call it "first_this" or "outer_that" or something ugly.
It's a cheap solution and it works but the risk is that the code becomes hard for humans to debug once it grows in scope.
Also Wrong But Better
var Increaser = function(amount) {
this.amount = amount;
};
Increaser.prototype.add = function(value) {
if (Array.isArray(value)) {
return value.map(function(item) {
return item + this.amount;
}.bind(this)); // NOTE!
} else {
return value + this.amount;
}
};
var inc = new Increaser(2);
console.log(inc.add(10)); // 12
console.log(inc.add([1, 2, 3])); // [3, 4, 5]
Why it's bad. Using .bind() creates a new function. It might not matter in this scenario but asking the JavaScript engine to create yet another function object in memory might matter in terms of optimization.
Why it's better. Because you "fix things" before it gets worse. This way, when deep inside the nested scope you don't need to juggle the name of what the this has temporarily been re-bound to.
Righter
The map function takes a second argument that is the context. This is true for forEach, filter and find too.
var Increaser = function(amount) {
this.amount = amount;
};
Increaser.prototype.add = function(value) {
if (Array.isArray(value)) {
return value.map(function(item) {
return item + this.amount;
}, this); // NOTE!
} else {
return value + this.amount;
}
};
var inc = new Increaser(2);
console.log(inc.add(10)); // 12
console.log(inc.add([1, 2, 3])); // [3, 4, 5]
Why it's better.
No new assignment of a variable. No need to bind, which means it doesn't create a new function. And it's built-in.
Much Righter
Switch to ES6! Then you can use fat arrow functions.
class Increaser {
constructor(amount) {
this.amount = amount;
}
add(value) {
if (Array.isArray(value)) {
return value.map(item => {
return item + this.amount;
});
} else {
return value + this.amount;
}
}
}
let inc = new Increaser(2);
console.log(inc.add(10)); // 12
console.log(inc.add([1, 2, 3])); // [3, 4, 5]
Why it's better: Because then the whole problem goes away. Fat arrow functions are functions that have no scope of their own. Just like if statements. (EDIT: That's an over simplification. They do have their own scope. Just no own arguments or own this)