2

I'm new to JavaScript. I'm trying to figure out why this doesn't work:

function myFunction(){
    document.getElementById("result").value=add(1,2);
}
function add(){
    var sum = 0;
    for(var i in arguments)
        sum += i;
    return sum;
}

This outputs 001. Why?

Xotic750
  • 22,914
  • 8
  • 57
  • 79
ConditionRacer
  • 4,418
  • 6
  • 45
  • 67

3 Answers3

9

You are iterating keys, do this:

function add(){
    var sum = 0;
    for(var i in arguments)
        sum += arguments[i];
    return sum;
}

More specifically, the keys are strings, "0" and "1", thus your response which is the concatenation of your initial 0, and subsequent keys.

Also, if you are interested in javascript on modern platforms, the following is very clear and concise.

function add(){
    return [].reduce.call(arguments, function(a, b) {
        return a+b;
    });
}
console.log(add(1,2));
matt3141
  • 4,303
  • 1
  • 19
  • 24
  • 5
    @Justin984: it's better form to use `for (var i = 0; i < arguments.length; i++)` instead since `for (var i in arguments` will iterate all properties of `arguments`, not just the indices. – beatgammit Jul 17 '13 at 01:40
  • @tjameson Can you give an example of what can go wrong? I'm not sure what you mean by iterating all properties. – ConditionRacer Jul 17 '13 at 01:44
  • @matt3141 Thanks for explaining this, unfortunately I'm a little too green for the second example to make much sense yet :P – ConditionRacer Jul 17 '13 at 01:45
  • @Justin984 I encourage you to take a look at `Array#reduce`, `Array#map`, etc. You can probably find a good article with them labeled as a group as "Functional JavaScript" – matt3141 Jul 17 '13 at 01:49
  • @Justin984 - You probably won't notice anything with `arguments`, any other object that has a prototype chain could cause problems. See [hasOwnProperty](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/hasOwnProperty) and [Douglas Crockford's tips](http://javascript.crockford.com/code.html) (his style is sometimes overly strict, but safe). – beatgammit Jul 17 '13 at 01:49
  • @Justin984 - For the functions on Array, see [mdn](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array). It's near the middle under methods, specifically accessor methods. – beatgammit Jul 17 '13 at 01:50
  • 1
    @tjameson If I'm not mistaken, the arguments object also has a prototype chain, it inherits from Object.prototype. – bfavaretto Jul 17 '13 at 02:22
  • @Justin984 Another problem with a `for...in` loop with `arguments` is that it doesn't seem to work in old IE. `arguments` is an array (in newer browsers) and an array-like object in older...normal `for` loops should be used to iterate over them. – Ian Jul 17 '13 at 15:28
  • @bfavaretto - I didn't notice anything weird in some tests on FF, but Chrome may be different. The short of the matter is that it can't be guaranteed and isn't a good pattern, especially since `for (;;)` works more reliably. – beatgammit Jul 17 '13 at 16:25
  • @tjameson Agreed, `for(;;)` is the right kind of loop for the arguments object. I tested in Chrome, and properties added to Object.prototype do show in a `for..in` loop over `arguments`. – bfavaretto Jul 17 '13 at 21:03
1

try this one:

function add(){
    var sum = 0, x = 0;
    for(var i in arguments){
        //the key is an index
        if(!arguments.hasOwnProperty(i)) continue;

        // try converting argument[i] to a number
        x = +arguments[i];

        // check if argument is a valid number 
        if(!isNaN(x)) {
            sum += x;
        }
    }
    return sum;
}

DEMO: http://jsfiddle.net/vg4Nq/

Ian
  • 50,146
  • 13
  • 101
  • 111
TheVillageIdiot
  • 40,053
  • 20
  • 133
  • 188
  • You don't really need `parseInt` here. – Blender Jul 17 '13 at 01:44
  • Why this: `typeof x != typeof NaN` ? The `typeof NaN` will always be `"number"`, and `typeof x` will also always be `"number"`, so the condition will never be met. –  Jul 17 '13 at 02:03
  • @matt3141 thanks for the update. Also I have moved the check for `hasOwnProperty` before `parseInt` as we had accessed `arguments[i]` already – TheVillageIdiot Jul 17 '13 at 02:38
  • @TheVillageIdiot Remember to provide the radix parameter to `parseInt`. And checking `if(isNaN(x))` means that you're checking that `x` is **not** a number...so you need to add `!`. I added both. Hopefully that's okay and is correct :) – Ian Jul 17 '13 at 02:41
  • @TheVillageIdiot I also just updated to actually convert to a number (not parse it...which can have problems, like if the value "5p" gets passed somehow), and added a demo – Ian Jul 17 '13 at 15:13
  • Also, this form of looping over `arguments` in old IE doesn't work :( Just another reason not to use `for...in` on arrays and array-like objects – Ian Jul 17 '13 at 15:19
0

Here is another possibility.

function add() {
    var numbers = [].slice.call(arguments),
        sum = 0;

    do {
        sum += numbers.shift();
    } while (isFinite(sum) && numbers.length);

    return +sum;
}

console.log(add(1, 2, 3, 4, 5));

On jsfiddle

Update: This is more cross browser than reduce. We can also improve on this if we wish to do a little more checking of the data.

function add1() {
    var numbers = [].slice.call(arguments),
        sum = 0,
        x;

    do {
        x = numbers.shift();
        sum += (typeof x === "number" ? x : NaN);
    } while (isFinite(sum) && numbers.length);

    return +sum;
}

console.log("add1", add1(1, 2, 3, 4, 5));
console.log("add1", add1(1, 2, 3, 4, 5, ""));
console.log("add1", add1(1, 2, 3, 4, 5, "0xA"));
console.log("add1", add1(1, 2, 3, 4, 5, NaN));
console.log("add1", add1(1, 2, 3, 4, 5, Infinity));
console.log("add1", add1(1, 2, 3, 4, 5, -Infinity));
console.log("add1", add1(1, 2, 3, 4, 5, true));
console.log("add1", add1(1, 2, 3, 4, 5, false));
console.log("add1", add1(1, 2, 3, 4, 5, null));
console.log("add1", add1(1, 2, 3, 4, 5, undefined));
console.log("add1", add1(1, 2, 3, 4, 5, []));
console.log("add1", add1(1, 2, 3, 4, 5, {}));

And we can compare that with no checking

function add2() {
    return [].reduce.call(arguments, function (a, b) {
        return a + b;
    });
}

console.log("add1", add2(1, 2, 3, 4, 5));
console.log("add1", add2(1, 2, 3, 4, 5, ""));
console.log("add2", add2(1, 2, 3, 4, 5, "0xA"));
console.log("add2", add2(1, 2, 3, 4, 5, NaN));
console.log("add2", add2(1, 2, 3, 4, 5, Infinity));
console.log("add2", add2(1, 2, 3, 4, 5, -Infinity));
console.log("add2", add2(1, 2, 3, 4, 5, true));
console.log("add2", add2(1, 2, 3, 4, 5, false));
console.log("add2", add2(1, 2, 3, 4, 5, null));
console.log("add2", add2(1, 2, 3, 4, 5, undefined));
console.log("add2", add2(1, 2, 3, 4, 5, []));
console.log("add2", add2(1, 2, 3, 4, 5, {}));

On jsfiddle

And how is the performance affected by these extra checks that were introduced?

Let's see, jsperf

Feel free to add the other solutions to the performance test. - I added the others.

Anyway, I would avoid using for..in when looping through an arguments object (to avoid looping through any methods that may have been attached to arguments) and would choose any of the other usual Array looping methods: for, while, forEach or reduce

See relevance: Why is using “for…in” with array iteration such a bad idea?

Community
  • 1
  • 1
Xotic750
  • 22,914
  • 8
  • 57
  • 79