Find and print all prime numbers (less than 10000)

Question

I want to improve my JS skills by doing challenges. My current challenge is: Write a script which allow user to enter number and then print all prime numbers from 0 to that number.

const PrimeFinder = (function() {

  const PRIMES = [];
  const MAX_RANGE = 10000;

  // cache DOM
  const EL_FIND = document.getElementById('find');
  const EL_RESULT = document.getElementById('result');
  const EL_SEARCH = document.getElementById('search');

  // bind events
  EL_FIND.addEventListener('click', findPrimes);

  function _renderResult() {
    for (let i = 0; i < PRIMES.length; i++) {
      const EL_PRIME = EL_RESULT.appendChild(document.createElement('div'));
      EL_PRIME.textContent = PRIMES[i];
    }
  }

  function _resetResult() {
    PRIMES.length = 0;
    EL_RESULT.textContent = '';
  }

  function _validUserInput(N) {
    return !isNaN(parseFloat(N)) &&
      isFinite(N) &&
      N >= 0 &&
      N <= MAX_RANGE;
  }

  function isPrime(n) {
    let start = 2;
    while (start <= Math.sqrt(n)) {
      if (n % start++ < 1) return false;
    }
    return n > 1;
  }

  function findPrimes(val) {
    const USER_INPUT = (typeof val === 'number') ? val : EL_SEARCH.value;
    _resetResult();

    if (_validUserInput(USER_INPUT)) {
      for (let i = 2; i <= USER_INPUT; i++) {
        if (isPrime(i)) PRIMES.push(i);
      }
      _renderResult();

      return PRIMES;
    }
  }

  const publicAPI = {
    isPrime: isPrime,
    findPrimes: findPrimes
  }

  return publicAPI;

})();

body,
html {
  padding: 0;
  margin: 0;
}

.container {
  max-width: 320px;
  margin: 0 auto;
}

#search,
#find {
  background: #fff;
  color: #000;
  height: 26px;
  line-height: 26px;
  margin: 0;
  padding: 0;
  box-sizing: border-box;
  border: 1px solid #666;
}

#find {
  width: 120px;
}

#search {
  width: 190px;
}

#result {
  margin: 10px 0;
}

#result>div {
  width: 20%;
  float: left;
  box-sizing: border-box;
  text-align: center;
  color: #fff;
  line-height: 48px;
  background: coral;
  border: 2px solid #fff;
}

#result div:hover {
  background-color: red;
}

<div class="container">
  <h1>Prime Number Finder</h1>
  <input placeholder="" id="search" type="number">
  <button id="find">Find</button>
  <div id="result"></div>
</div>

I would like to know what could be done better and why.

Answer 1

This is a very nice start! It is clear that you have put some effort into making your code clear and easy to read - it paid off. That said, there are a few things I would recommend changing.

1. Don't take input as a float. This opens up the possibility for confusion without providing any benefit to the user.
2. Include USER_INPUT in the range tested for primes. If I input 5 using the form I only get 2 and 3, while I expected 2, 3, and 5.
3. Why do you cache PRIMES only to reset it every time findPrimes is called? If you are going to spend the memory to save a list of primes, you might as well make use of it.
4. I see you have started to optimize your solution by only going up to Math.sqrt(n), but you are still using an inefficient algorithm. I would encourage you to look into the Sieve of Eratosthenes as your input is bounded.
5. Don't mix your logic and display code. While for a small project like this it isn't much of an issue, if you want to use PrimeFinder somewhere else in your application you would have to rewrite it to strip out all of the page specific code.

---

With this in mind, here is how I would write PrimeFinder as a module without handling DOM manipulations.

const PrimeFinder = (function() {
    let PRIMES = [];
    const MAX_RANGE = 10000;

    function _generatePrimes() {
        // Computing 10000 is not much different than computing 500, so fill the entire array.
        let marks = Array(MAX_RANGE + 1).fill(false)
        for (let i = 2; i <= MAX_RANGE; i++) {
            // Already marked as non-prime?
            if (marks[i]) continue;
            // Mark all multiples
            for (let j = i * 2; j <= MAX_RANGE; j += i) {
                marks[j] = true;
            }
            // This number hasn't been marked, and therefore is prime
            PRIMES.push(i);
        }
    }

    function isPrime(n) {
        if (Number.isNaN(n)) throw Error('n must be a number');
        if (n > MAX_RANGE || n < 0) throw Error('n is out of range.');
        if (!PRIMES.length) _generatePrimes();
        // Could be sped up with a binary search.
        return PRIMES.includes(n);
    }

    function findPrimes(upperBound) {
        if (Number.isNaN(upperBound)) throw Error('upperBound must be a number');
        if (upperBound > MAX_RANGE) throw Error(`upperBound must be less than ${MAX_RANGE}`);
        if (upperBound < 0) throw Error('upperBound must be at least 0');

        if (!PRIMES.length) _generatePrimes();

        let index = PRIMES.findIndex(n => n > upperBound);
        // Return a copy of the array so users can't mess with the module
        return index == -1 ? [...PRIMES] : PRIMES.slice(0, index);
    }

    return { isPrime, findPrimes };
}());

function displayArray(parent, items) {
  parent.innerHTML = '';
  items.forEach(value => {
    let el = document.createElement('li');
    el.textContent = value;
    parent.appendChild(el);
  });
}

document.querySelector('input').addEventListener('input', event => {
  let num = parseInt(event.target.value, 10);
  if (num > 10000) event.target.value = num = 10000;
  if (num < 0 || Number.isNaN(num)) event.target.value = num = 0;
  displayArray(
    document.querySelector('ul'),
    PrimeFinder.findPrimes(num)
  );
});

<input type="number" />
<ul></ul>

---

The snippet behaves similarly to the provided snippet without mixing DOM logic into the PrimeFinder functionality - this makes it possible to easily pull out PrimeFinder for use in other projects (even those where the DOM might not exist).