SpencerLS
SpencerLS

Reputation: 371

Prime checking program not working on certain numbers

I was excited because my prime number program was working, but when I tried certain numbers like 5 and 10, 2 and 11, it wasn't working. I have tried to figure it out myself, and by looking at other things related to what I was doing, but I couldn't figure it out. I would appreciate an answer and thank everyone who helps, because I am just a beginner at coding and want to learn more.

var minimum;
var maximum;
var primes;

function myFunction(e) {
  e.preventDefault();
  minimum = document.getElementById('Minimum').value;
  maximum = document.getElementById('Maximum').value;
  primes = [];

  listOfPrimes(minimum, maximum); 
  primes = primes.filter(function(x) {
    return x > 1;
  });
  primes = primes.toString();
  primes = primes.replace(/,(?=[^\s])/g, ", ");
  document.getElementById("Your_Primes").innerHTML = primes;
}

function isPrime(num) {
  for (var i = 2; i <= num / 2; i++)
    if (num % i === 0) {
      return false;
    }
  primes.push(num);
}

function listOfPrimes(min, max) {
  for (var j = min; j <= max; j++) {
    isPrime(j);
  }
}
body {
	background-color: #dbdbdb;
}

#prime-checker {
	background-color: #89a9dd;
	margin: 5px;
	padding: 5px;
	text-align: center;
}

.the-primes {
	background-color: #587cb7;
	padding: 15px;
	margin: 10px;
}
<!doctype html>

<html>
<head>
	<title>Prime Lister</title>
	<link href="main.css" rel="stylesheet" type="text/css">
</head>
<body>

<div id="prime-checker">

<form>
  <h2>Minimum Number:</h2><br>
  <input type="number" placeholder="MinimumWholeNumber" id="Minimum" multiple="1" min="0" required><br><br> 
  <h2>Maximum Number:</h2><br>
  <input type="number" placeholder="MaximumWholeNumber" id="Maximum" multiple="1" min="0" required><br><br><br>
  <button id="button" onclick="myFunction(event)">Enter</button>
</form>


<div class="the-primes"><h2>Your Primes:</h2>
  <p id="Your_Primes"></p>
</div>

</div>

<script type="text/javascript" src="Web_Prime_Lister.js"></script>
</body>
</html>

Upvotes: 1

Views: 79

Answers (2)

Got it! Simply change this line:

  for (var j = min; j <= max; j++) {

to this:

  for (var j = parseInt(min); j <= parseInt(max); j++) {

Get familiar with Debugger Tools (F12 in most browsers). There you can pause execution in certain lines and check value and type of variables. If you do it, there you'll see than min = "5", insted of min = 5. That's why 5-49 doesn't work, beacuse "5" is smaller than "49" (beacuse it's first digit is lower), so it doesn't loop at all.

Upvotes: 1

CRice
CRice

Reputation: 32186

Your code for calculating a prime looks fine. The problem is that you are using the raw strings from the minimum and maximum to create your range. The problem is that strings are compared lexigraphically, so:

"2" < "100" // false

So when you pass those values into your function listOfPrimes, that loop will terminate early due to the string comparison.

To fix it, just convert your input values to numbers before you use them:

minimum = Number(document.getElementById('Minimum').value);
maximum = Number(document.getElementById('Maximum').value);

Here's your snippet with that fix, everything looks like it's working well:

var minimum;
var maximum;
var primes;

function myFunction(e) {
  e.preventDefault();
  minimum = Number(document.getElementById('Minimum').value);
  maximum = Number(document.getElementById('Maximum').value);
  primes = [];

  listOfPrimes(minimum, maximum); 
  primes = primes.filter(function(x) {
    return x > 1;
  });
  primes = primes.toString();
  primes = primes.replace(/,(?=[^\s])/g, ", ");
  document.getElementById("Your_Primes").innerHTML = primes;
}

function isPrime(num) {
  for (var i = 2; i <= num / 2; i++)
    if (num % i === 0) {
      return false;
    }
  primes.push(num);
}

function listOfPrimes(min, max) {
  for (var j = min; j <= max; j++) {
    isPrime(j);
  }
}
body {
	background-color: #dbdbdb;
}

#prime-checker {
	background-color: #89a9dd;
	margin: 5px;
	padding: 5px;
	text-align: center;
}

.the-primes {
	background-color: #587cb7;
	padding: 15px;
	margin: 10px;
}
<!doctype html>

<html>
<head>
	<title>Prime Lister</title>
	<link href="main.css" rel="stylesheet" type="text/css">
</head>
<body>

<div id="prime-checker">

<form>
  <h2>Minimum Number:</h2><br>
  <input type="number" placeholder="MinimumWholeNumber" id="Minimum" multiple="1" min="0" required><br><br> 
  <h2>Maximum Number:</h2><br>
  <input type="number" placeholder="MaximumWholeNumber" id="Maximum" multiple="1" min="0" required><br><br><br>
  <button id="button" onclick="myFunction(event)">Enter</button>
</form>


<div class="the-primes"><h2>Your Primes:</h2>
  <p id="Your_Primes"></p>
</div>

</div>

<script type="text/javascript" src="Web_Prime_Lister.js"></script>
</body>
</html>


PS: You can further optimize your isPrime function by only checking divisors up to Math.floor(Math.sqrt(num)). That will save you many many iterations, especially for a large number. So:

function isPrime(num) {
  var max = Math.floor(Math.sqrt(num));
  for (var i = 2; i <= max; i++)
    if (num % i === 0) {
      return false;
    }
  primes.push(num);
}

As you can see, for a 10-digit prime, the running time is very much reduced:

function isPrimeSqrt(num) {
  var max = Math.floor(Math.sqrt(num));
  for (var i = 2; i <= max; i++)
    if (num % i === 0) {
      return false;
    }
  return true;
}

function isPrimeHalf(num) {
  var max = Math.floor(num / 2);
  for (var i = 2; i <= max; i++)
    if (num % i === 0) {
      return false;
    }
  return true;
}

let now = Date.now();
const pHalf = isPrimeHalf(1010189899);
console.log("Using 'Half' took:", Date.now() - now, "ms. Is prime:", pHalf);
now = Date.now()
const pSqrt = isPrimeSqrt(1010189899);
console.log("Using 'Sqrt' took:", Date.now() - now, "ms. Is prime:", pSqrt);

Upvotes: 6

Related Questions