Trouble sorting an array of numbers into evens and odds

Question

I'm just learning JavaScript and am working on a problem where I sort an array of numbers into odd and even arrays.

To test whether I am successful, I am "alerting" the arrays at the end (and expect to see the even numbers in the first alert, the odd numbers in the second alet). However, everything I try hasn't worked; I just get an alert with 1, then an empty alert.

Can anyone tell me why this is not working?

var numbs = [1, 2, 34, 54, 55, 34, 32, 11, 19, 17, 54, 66, 13];
Sort(numbs);

function Sort(nums) {

    var evens = [];
    var odds = [];
    for (var i = 0; i < arguments.length; i++) {

        if (nums[i] % 2) {
            evens.push(nums[i]);
        } else {
            odds.push(nums[i]);
        }
    }
    alert(evens.toString());
    alert(odds.toString());
}

Answer 1

You need to make a couple fixes (and should clean up your code's formatting a bit for readability & maintainability – like I have done below):

var numbs = [1,2,34,54,55,34,32,11,19,17,54,66,13];
Sort(numbs);

function Sort(nums) {
    var evens = [];
    var odds = [];

    //alert(arguments.length); // for debugging
    for (var i = 0; i < nums.length; i++) { // nums.length per commentary below
    //for (var i = 0; i < arguments.length; i++) {
        if (nums[i]%2){ 
            odds.push(nums[i]); // odds, not evens per commentary below
            //evens.push(nums[i]);
        }
        else {
            evens.push(nums[i]); // evens, not odds per commentary below
            //odds.push(nums[i]);
        }
    }

    alert(evens.toString());
    alert(odds.toString());
}

arguments.length is the number of arguments passed to the function, not the length of the array passed as a single argument in this case (i.e. arguments[0].length); further, there is little reasons to use the arguments array at all when you can refer to a named parameter (i.e. nums).

Also, your evens/odds logic is flip-flopped.

Answer 2

The answer from @Doorknob 冰 captures very well the essence of what went wrong, and describes how you can go about fixing it.

But if you're interested in a slightly more advanced technique, then this code will work, and it is, to my mind much more elegant:

var isEven = function(n) {return n % 2 === 0;};
var isOdd = function(n) {return n % 2 !== 0;};

numbs.filter(isEven); //=> [2, 34, 54, 34, 32, 54, 66]
numbs.filter(isOdd); //=> [1, 55, 11, 19, 17, 13]

In other words, the work you're doing to filter the items is already a built-in method on the array prototype. You just need to define predicate functions to use with it, such as isEven and isOdd.

(Note that the test in isOdd is not n % 2 === 1 since the % operator is slightly different from the mathematical modulus operation, and will return a negative value if the dividend is negative.)

Answer 3

Try the following code, it takes the array, numbs, and sorts it and inserts the odd numbers in the odd array, and the even numbers into the even array:

var numbs = [1,2,34,54,55,34,32,11,19,17,54,66,13];
Sort(numbs);

var evens = [];
var odds = [];

function Sort(nums) {

    for (var i = 0; i < nums.length; ++i) {
        if ((nums[i] % 2) === 0) {
            evens.push(nums[i]);
        }
        if ((nums[i] % 2) == 1) {
            odds.push(nums[i]);
        }
    }
    alert(evens);
    alert(odds);
}

Answer 4

You were pretty close you just have to make a couple of changes

The first change needs is to change the arguments.length to nums.length since you are passing the array as a parameter you are using its length.

The second change needed is to change the following in your if then statement:

   nums[i]  % 2

to:

  nums[i]  % 2 === 0

Currently you are not storing even numbers in your evens array.

javascript:

    var numbs = [1,2,34,54,55,34,32,11,19,17,54,66,13];
Sort(numbs);
function Sort(nums) {

var evens = [];
var odds = [];
for (var i = 0; i < nums.length; i++) {//<---change number 1

if (nums[i]  % 2 === 0){ //<--- change number 2 if the remainder = 0 it is even
        evens.push(nums[i]);
      }
      else {
        odds.push(nums[i]);
      }
    }
alert(evens.toString());
alert(odds.toString());
}

Answer 5

for (var i = 0; i < arguments.length; i++) {
                    ^^^^^^^^^^^^^^^^

This is your problem. arguments refers to the array of all arguments passed to the function (ex. the arguments value for function(a, b, c) is [a, b, c]). Therefore, in your case, arguments.length will be 1 since there is only one argument.

In fact, as you may have gathered, the use of arguments is often unnecessary. Typically you can just check if the arguments are undefined if you want to know whether they have been provided or not, and the use of arguments will slow down your code drastically. You should probably almost never have to use arguments in your code at all.

Simply replace that line with:

for (var i = 0; i < nums.length; i++) {

---

A few other observations:

• Your if condition is flipped—it detects whether a number is odd, not even. Either flip the bodies of the if and else statements or use something like nums[i] % 2 !== 1 instead.

• You use numbs as a local variable before the function call, and then call it nums in the function. Why? This seems odd and confusing, and standardizing on a single name will help reduce mistakes in the future. (You could also call them, say, userAges outside the function and nums inside if you're passing something specific to it.)

• Your function is called Sort with a capital S. This is unusual because the standard naming conventions for JavaScript dictate that variables and functions be camelCased, i.e. starting with a lowercase letter and having uppercase letters to distinguish new words.

• The indentation seems to be quite erratic. This makes your code harder to read. Try using an IDE or some sort of text editor that will automatically indent your code for you, making it much simpler to read.