To 'this' or not to 'this'?

I was given a homework and I have 2 solutions: one that uses this and other one that doesn't.

I tested both solutions on jsPerf but sometimes it says the version with this is faster or sometimes the other one. So I think I cannot really rely on jsPerf. Anyway, should performance be always the top priority?

By the way, here is the problem statement:


Write a function addg that adds from many invocations, until it sees an empty invocation.


Something like:


addg(); // undefined


addg(2)(); // 2


addg(2)(7)(); // 9


addg(3)(0)(); // 3


addg(1)(2)(4)(8)(); // 15


addg(5)(4)(-2)(); // 7

Here are my 2 solutions:

• version with this:

function add(a,b){
return a+b;
}

function addg(val){
var numbers = [];

this.addToCollection = function(val){
numbers.push(val);

if(typeof val !== 'number'){ // in case of ()
var sum = 0,
l = numbers.length;

numbers.forEach(function(num, i){
if(i === l-1){
return;
} else {
sum = add(sum, num);
}
});

// console.log(numbers);

if(numbers.length === 1 && numbers[0] === undefined){
return undefined;
} else {
return sum;
}

} else { // in case of (n)
return this.addToCollection; // return the function each time so you can apply it to other numbers in the chain
}

};

return this.addToCollection(val);
}

console.log(addg()); // undefined
console.log(addg(2)()); // 2
console.log(addg(2)(7)()); // 9
console.log(addg(3)(0)()); // 7
console.log(addg(1)(2)(4)(8)()); // 15
console.log(addg(5)(4)(-2)()); // 7

• version without this:

`function add(a,b){
return a+b;
}

function addg(val){
var numbers = [];

function addValues(val){
numbers.push(val);

if(typeof val !== 'number'){ // in case of ()

First and foremost:


Anyway, should performance be always the top priority?

Definitely not. Don't write things that are needlessly unperformant, but only be overly concerned with performance when you have a reason to be. Most of the time JS execution time is irrelevant, because you're waiting for user interaction, or for a network request, or something else that makes the execution time of the JS pretty unimportant.

I'd be more concerned with writing code that's clear, expressive, and maintainable than performant.

Regarding your usage of this, though, you definitely shouldn't be using this as you are in the first example. You're leaking into the global scope. When addg is run, what is this? If you were calling obj.addg(), this would be obj, but since you're just calling addg(), this is the global scope A.K.A. window (in the browser).

With your first version if you run this in your console you'll see:

addg(); 
//undefined
addToCollection;
//function!  Whoops

That's not what you want. You don't need to assign the internal addToCollection function anywhere, so don't.

An add function is a bit pointless. You're adding undefined to the array of numbers in the () case, then adding special logic to exclude it from the sum, later, but instead you should just not add undefined at all. Only do numbers.push(val) if typeof val === 'number'.

But, then, do you really need to keep an array of numbers at all?

StackExchange Code Review Q#112828, answer score: 29