JS: Always get empty array?

Question

I am trying to loop between two numbers and to return all prime numbers between them. In the first function isNumberPrime, I check if the number is prime and return 0 if it is not and 1 if it is prime. In the second function I am trying to loop between two numbers and to check if the flag is = 1, I push the number in the array. But, I can not see where the problem is?

function isNumberPrime(flag) {
    let number;
    let divider;
    if ((number != 2 || number%2 === 0) || number === 1){
        flag = 0; //not prime number 0
    }
    else {
        divider = 3;    
        while(divider <= number/2){
            if (number%divider === 0){
                falg = 0;
    
                break;
            }
            divider = divider + 2;
        }
        if(divider > number/2){
            flag = 1;
        }
    }

    return flag;
}

function looping(lowerNumber, higherNumber) {
    let allPrimes = [];
    for (let i = lowerNumber; i <= higherNumber; i++) {
        isNumberPrime(i);
        if (isNumberPrime(i) == 1){
            allPrimes.push[i];
        }
    }

    return allPrimes;
}

function enterNumbers() {
    let input1 = document.getElementById("firstNumber");
    let lowerNumber = input1.value;

    let input2 = document.getElementById("secondNumber");
    let higherNumber = input2.value;

    let output = document.getElementById("output");

    output.innerText = 'All primes between ' + lowerNumber +' and ' + higherNumber + ' are: [' + looping(lowerNumber, higherNumber) + ']';
}

const button = document.querySelector('button');
button.addEventListener('click', enterNumbers)

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Document</title>
</head>
<body>
  <h1>Prime numbers</h1>
  <p>Type first number and second number:</p>
  <div>
    <input id="firstNumber" type="number" size="24" />
    <input id="secondNumber" type="number" size="24" />
    <button>Show primes</button> <br />
  <span id="output"></span>
  </div>
  <script src="primeNumber.js"></script>
</body>
</html>

There is a typo in the code: falg is used instead of flag. See line after if (number%divider === 0) {. I hope it helps — Dmitry Messerman, Sep 28 '21 at 04:49

Nitheesh · Accepted Answer · 2021-09-28T05:59:34.747

I have identified and fixed below issues with your code.

Issues

In isNumberPrime function, you expect flag as a function prameter. Thats wrong. The parameter should be the number
There is a typo error flag was misspelled as falg inside isNumberPrime
The condition number != 2 will assume all the numbers other than 2 as not prime. So removed that.
Also the syntax for Array.push is wrong. it should be allPrimes.push(i) and not allPrimes.push[i]
I have made the lower boundary and upper boundary as the max value between input value and 0 to avoid negative numbers.
Also I have used Math.ceil() in the starting number to take number greater than or equal to input and Math.floor() in ending number to take number less than or equal to input in case of decimals.
I have updated looping function call as looping(+lowerNumber, +higherNumber), to make the parameter number. The value fetched from an input will be string by default.
If you want the element to be inside the form, you can wrap the elements inide a form. But a button inside a form will be treated as a submit button. So you have to explicitly make the button type="button" or you have to preventDefault on form submit. I went for the second approach in the updated fiddle.

Working Fiddle

function isNumberPrime(number) {
  let flag;
  let divider;
  // Wrong: number != 2 will assume all the numbers other than 2 as not prime
  if (number % 2 === 0 || number === 1) {
    flag = 0; //not prime number 0
  }
  else {
    divider = 3;
    while (divider <= number / 2) {
      if (number % divider === 0) {
        flag = 0; // Typo error fix

        break;
      }
      divider = divider + 2;
    }
    if (divider > number / 2) {
      flag = 1;
    }
  }
  return flag;
}

function looping(lowerNumber, higherNumber) {
  let allPrimes = [];
  const lowerBoundary = Math.ceil(Math.max(lowerNumber, 0));
  const higherBoundary = Math.floor(Math.max(higherNumber, 0));
  for (let i = lowerBoundary; i <= higherBoundary; i++) {
    const isPrime = isNumberPrime(i);
    if (isPrime == 1) {
      // Wrong syntax of `Array.push` fixed
      allPrimes.push(i);
    }
  }
  return allPrimes;
}

function enterNumbers() {
  let input1 = document.getElementById("firstNumber");
  let lowerNumber = +input1.value;
  let input2 = document.getElementById("secondNumber");
  let higherNumber = +input2.value;
  let output = document.getElementById("output");
  if (higherNumber > lowerNumber) {
    output.innerText = 'All primes between ' + lowerNumber + ' and ' + higherNumber + ' are: [' + looping(lowerNumber, higherNumber) + ']';
  } else {
    output.innerText = 'Lower boundary should be smaller than the higher';
  }
}
const button = document.querySelector('button');
button.addEventListener('click', enterNumbers);

function submitForm(event) {
  event.preventDefault();
}
const form = document.querySelector('form');
form.addEventListener('submit', submitForm);

<h1>Prime numbers</h1>
<p>Type first number and second number:</p>
<form>
  <input id="firstNumber" type="number" size="24" />
  <input id="secondNumber" type="number" size="24" />
  <button>Show primes</button> <br />
  <span id="output"></span>
</form>

Thank u so much. This works now how it should. Do you know maybe know how to add form tag outside of div and to still have output, not disappear immediately after click button? — M Alex, Sep 28 '21 at 05:20
If you use -1 as starting number you get the wrong result. See my answer for a working algorithm (feel free to copy it). Also use parseInt() instead of + to convert number as I did otherwise 1.1 to 10 would give incorrect result. — Allan Wind, Sep 28 '21 at 05:23
a to 12 also gives wrong answer, and displayed text is iffy. — Allan Wind, Sep 28 '21 at 05:41
1.1 to 10 and a to 10 gives an error message but still show wrong results). 1.1 either coerce it with parseInt() or reject it as invalid input but then don't run invalid data through isNumberPrime(). — Allan Wind, Sep 28 '21 at 05:46
a to 10 still doesn't work quite right. You get error message but still does calculation as if you entered 0 instead of "a". The fix I figured out was to check if Number(text) =="" — Allan Wind, Sep 28 '21 at 06:01
@AllanWind If the start s not defined, I treat this as 0. I dont thik this as a bug — Nitheesh, Sep 28 '21 at 06:02
The bug is that error message (Please enter a number) but you still do the calculation. If there was no error message then it would be fine as you say, or if there as an error message and no result that would be good too. Btw, upvoted for effort. See my answer, not as pretty error handling as your answer, but I think the behavior is solid. — Allan Wind, Sep 28 '21 at 06:03
@AllanWind in the question perspective, we have covered the issues that he is facing with. And this doesnot seems to be breaking. I have not added any form validation in the fiddle, and hence its not throwing an error. If we start to fix each and every bug that have been identiffied this wont end aywhere. Because every person will have different point of view wrt solution. So, I'm not making any change to current implementation, because the requirement of the user has already been satisafied, and he approved the answer. — Nitheesh, Sep 28 '21 at 06:51
@Nitheesh no worries (or any pressure to fix stuff), just noticed it when I worked on my answer and saw it applied to yours as the accepted answer. — Allan Wind, Sep 28 '21 at 06:54

Allan Wind · Answer 2 · 2021-09-28T06:09:57.757

isNumberPrime() algorithm was incorrect, also you had it take a flag instead of number argument.
looing() used push[i] used instead of push(i).
enterNumbers() passed two strings to looping() instead of integers; aggressive type checks in logging().

function isNumberPrime(number) {
    if(number < 2) return false;
    for(let i = 2; i <= number / 2; i++) {
       if(!(number % i)) return false;
    }
    return true;
}

function looping(lowerNumber, higherNumber) {
    if(lowerNumber == "" || !Number.isInteger(lowerNumber) || lowerNumber < 0 || higherNumber == "" || !Number.isInteger(higherNumber) || higherNumber < 0) {
       console.error("looping is called with invalid arguments");
       return [];
    }
    let allPrimes = [];
    for(let i = lowerNumber; i <= higherNumber; i++) {
        if(isNumberPrime(i)) {
            allPrimes.push(i);
        }
    }
    return allPrimes;
}

function enterNumbers() {
    let input1 = document.getElementById("firstNumber");
    let lowerNumber = input1.value;

    let input2 = document.getElementById("secondNumber");
    let higherNumber = input2.value;

    let output = document.getElementById("output");
  
    output.innerText = 'All primes between ' + lowerNumber +' and ' + higherNumber + ' are: [' + looping(Number(lowerNumber), Number(higherNumber)) + ']';
}

const button = document.querySelector('button');
button.addEventListener('click', enterNumbers)

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Document</title>
</head>
<body>
  <h1>Prime numbers</h1>
  <p>Type first number and second number:</p>
  <div>
    <input id="firstNumber" type="number" size="24" />
    <input id="secondNumber" type="number" size="24" />
    <button>Show primes</button> <br />
  <span id="output"></span>
  </div>
  <script src="primeNumber.js"></script>
</body>
</html>

Not sure why it worked sometimes, but the issue was that you pass two strings to `looping()` instead of two integers. — Allan Wind, Sep 28 '21 at 05:19

Alireza Ahmadi · Answer 3 · 2021-09-28T05:21:28.303

You have some problems as follows:

First push methods are used in wrong way and the right way is allPrimes.push(i)
Second you are using number while you didn't initialize the variable, so initialize it let number = flag;
Third you need to check number === 2 instead of number != 2.
lowerNumber and higherNumber is string, so convert it to number by + operation

Here is working sample:

function isNumberPrime(flag) {
    let number = flag;
    let divider;
    if ((number === 2 || number%2 === 0) || number === 1){
        flag = 0; //not prime number 0
    }
    else {
        divider = 3;    
        while(divider <= number/2){
            if (number%divider === 0){
                falg = 0;
    
                break;
            }
            divider = divider + 2;
        }
        if(divider > number/2){
            flag = 1;
        }
    }

    return flag;
}

function looping(lowerNumber, higherNumber) {
    let allPrimes = [];
    debugger
    for (let i = +lowerNumber; i <= +higherNumber; i++) {
        if (isNumberPrime(i) == 1){
            allPrimes.push(i);
        }
    }

    return allPrimes;
}

function enterNumbers() {
    let input1 = document.getElementById("firstNumber");
    let lowerNumber = input1.value;

    let input2 = document.getElementById("secondNumber");
    let higherNumber = input2.value;

    let output = document.getElementById("output");

    output.innerText = 'All primes between ' + lowerNumber +' and ' + higherNumber + ' are: [' + looping(lowerNumber, higherNumber) + ']';
}

const button = document.querySelector('button');
button.addEventListener('click', enterNumbers)

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Document</title>
</head>
<body>
  <h1>Prime numbers</h1>
  <p>Type first number and second number:</p>
  <div>
    <input id="firstNumber" type="number" size="24" />
    <input id="secondNumber" type="number" size="24" />
    <button>Show primes</button> <br />
  <span id="output"></span>
  </div>
  <script src="primeNumber.js"></script>
</body>
</html>

This does not work if you type 2 and 11, you get empty array? — M Alex, Sep 28 '21 at 05:06
Thanks, now works. Do you know maybe know how to add form tag outside of div and to still have output, not disappear immediately after click button? — M Alex, Sep 28 '21 at 05:22
You need to add `change` event and call your methods if both text box has values — Alireza Ahmadi, Sep 28 '21 at 05:25

score 0 · Answer 4 · edited Sep 28 '21 at 05:59

You have a few issues here: In your isNumberPrime function:

You are returning flag while it is also the input param of the function. and you are checking number while it was only declared but never assigned any value.
There is an issue with the condition which is why every number you will input will result in flag being equals to 0. So instead of using and OR operator you should use AND.
You have a typo in the else statement falg instead of flag.

So it should look like this:

function isNumberPrime(number) {
  //switched between the flag and number
  let flag;
  let divider;
  if (
    (number != 2 && number % 2 === 0) || //changed || to &&
    number === 1
  ) {
    flag = 0;
  } else {
    divider = 3;
    while (divider <= number / 2) {
      if (number % divider === 0) {
        flag = 0; //falg to flag
        break;
      }
      divider = divider + 2;
    }
    if (divider > number / 2) {
      flag = 1;
    }
  }

  return flag;
}

In the looping function:

You used square brackets push[i] when pushing the values into the array. You need to use round brackets push(i)

JS: Always get empty array?

4 Answers4