Trouble pushing to an array in JS

Question

Below is just a section of my code but I know it's problematic because I can't get it to return any value except 'undefined'. I have been over this for hours and cannot figure it out.

I want to be able to input a number and have its factors pushed to an array. I have tested it by alerting the first item in the array and I get nothing. I'm sure this is a pretty easy but I just can't figure it out. Here is the code:

    var numberInQuestion = prompt("Of what number are you wanting to find the largest        prime factor?");

    //determine factors and push to array for later use
    var factorsArray = [];
    function factors(numberInQuestion){
        for(var i = 2; i < numberInQuestion-1; i++){
            if(numberInQuestion % i === 0){
                return factorsArray.push[i];
            } else {
                continue;
            }
        }
    };
    factors(numberInQuestion);
    alert(factorsArray[0]);

Thanks for any help!

The prompt is a little bit deceiving since the function does not check for a prime and it actually returns the smallest factor. — Kevin Bowersox, Sep 01 '13 at 15:41

tckmn · Answer 1 · 2013-09-01T15:33:58.993

2

you can only return one value
you must use (), not [] for calling push
factorsArray should be local to factors (put the definition inside the function)
the else { continue; } is useless

Here is the fully corrected code:

var numberInQuestion = prompt("Of what number are you wanting to find the factors of?");

//determine factors
function factors(numberInQuestion){
    var factorsArray = []; // make it local
    for (var i = 2; i < numberInQuestion-1; i++){
        if(numberInQuestion % i === 0){
            factorsArray.push(i); // use (), and don't return here
        } // no need for else { continue; } because it's a loop anyway
    }
    return factorsArray; // return at the end
};
var result = factors(numberInQuestion); // assign the result to a variable
alert(result);

Here's a JSFiddle.

edited Sep 01 '13 at 15:33

answered Sep 01 '13 at 15:28

tckmn

57,719
27
114
156

It would probably also be a good idea to make "factorsArray" be a local variable of the function. – Pointy Sep 01 '13 at 15:29
1

@Doorknob: Nope. [Array.push](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/push) returns the new length of the array. – MD Sayem Ahmed Sep 01 '13 at 15:30
To be precise, it's returning undefined because the function push has no property named "2". – bfavaretto Sep 01 '13 at 15:33
Thanks so much, this really helped a lot. It's awesome how the programming community helps out like this! – michaelto20 Sep 01 '13 at 15:35

MD Sayem Ahmed · Answer 2 · 2013-09-01T15:34:26.793

You have an error in your pushing syntax. Correct syntax for pushing is -

factorsArray.push(i);

Also returning immediately from the function after finding the first divisor will not give you the full list. You probably want to return after you've found out all the divisors.

Taking all of the above into consideration, you should rewrite your function as follow -

function factors(numberInQuestion){

    for(var i = 2; i < numberInQuestion - 1; i++){
        if(numberInQuestion % i === 0) {
            factorsArray.push(i);
        }
    }
}

and you will be OK.

score 0 · Answer 3 · answered Sep 01 '13 at 15:27

You've coded this so that when you find the first factor your function returns immediately. Just get rid of the return keyword in that statement. (What "return" means in JavaScript and other similar languages is to immediately exit the function and resume from where the function was called.)

Oh, also, you call functions (like .push()) with parentheses, not square brackets.

Kevin Bowersox · Answer 4 · 2013-09-01T15:49:22.320

The function should not return when pushing to the array. Return the array after executing the loop. The else clause is also unnecessary.

var numberInQuestion = prompt("Of what number are you wanting to find the largest        prime factor?");

function factors(numberInQuestion){
    var factorsArray = [];
    for(var i = 2; i < numberInQuestion-1; i++){
        if(numberInQuestion % i === 0 && isPrime(i)){
            factorsArray.push(i);
        }
    }
    return factorsArray; 
};
var factors = factors(numberInQuestion);
alert(factors[factors.length-1]);

//From: http://stackoverflow.com/questions/11966520/how-to-find-prime-numbers
function isPrime (n)
{
    if (n < 2) return false;
    var q = Math.sqrt (n);

    for (var i = 2; i <= q; i++)
    {
        if (n % i == 0)
        {
            return false;
        }
    }

    return true;
}

Given the purpose of the example two items must be considered

The code does not determine if the number is actually prime. The code will return the smallest factor possible since the loop starts at two and increments, then returns the first element in the array. The largest factor would actually be the last element in the array. I have corrected the example to find the greatest prime factor. You can test it via this fiddle: http://jsfiddle.net/whKGB/1/

Thanks for taking a look and helping out. – michaelto20 Sep 01 '13 at 15:36 — michaelto20, Sep 01 '13 at 15:36
@michaelto20 Glad I could help please see comments – Kevin Bowersox Sep 01 '13 at 15:45 — Kevin Bowersox, Sep 01 '13 at 15:45

Trouble pushing to an array in JS

4 Answers4