SPOILER! My code is 5x slower than solutions. Why?

Tell us what’s happening:
My solution runs way slower than any of the provided solutions. Can someone explain or provide a reference as to why?

  **Your code so far**

function sumPrimes(num) {
console.time('start')
const primeArr = []

for(let primes = 2;primes<=num;primes++){
let divisorCount = 0
while(divisorCount <= 2){
  for(let prime = 1;prime<=primes;prime++){
    if(primes%prime == 0){
    divisorCount++;
      }
    }
if(divisorCount==2){
  primeArr.push(primes)}
  }  
}
console.timeEnd('start')
return primeArr.reduce((prev,current)=>prev+current)

}

  **Your browser information:**

User Agent is: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:97.0) Gecko/20100101 Firefox/97.0

Challenge: Sum All Primes

Link to the challenge:

I did small adjustments and it went from ~80ms to ~15ms

function sumPrimes(num) {
  console.time('start')
  const primeArr = []

  for (let primes = 2; primes <= num; primes++) {
    if (primes > 2 && primes % 2 === 0) continue; // skip if even
    let divisorCount = 0
    // while (divisorCount <= 2) { // no need for while loop
    for (let prime = 3; prime < primes; prime++) { // even numbers >2 are already excluded
      if (primes % prime == 0) {
        divisorCount++
        break // we found a divisor, break out of loop
      }
    }
    if (!divisorCount) { // there was no divisors, it's a prime
      primeArr.push(primes)
    }
    // }
  }
  console.timeEnd('start')
  return primeArr.reduce((prev, current) => prev + current)
}

You can also replace primeArr with number for a bit more optimization.

1 Like

Please remember to use spoiler tags when asking questions about a working solution!

Since your code works, I would look closely at the solutions and see where they are different.

  1. Don’t store thing in an array if you only need them once. Building an array is expensive. Here you can just sum values as you find them.

  2. Don’t compute more than you need to. A number only has unique divisors up to the square root, s you shouldnt check numbers past the square root for divisability.

Also note, Javascript isn’t a high performance language, so timings and performance comparison can be a bit tricky. For more, see the discussion here:

1 Like

:slightly_smiling_face: Thanks for all of your help and insight. Seems an elementary math lesson is in order. I think I have implemented both of your suggestions. The code runs way faster. Thank you.

function sumPrimes(num) {
  console.time('better?');
  if(num <2){ /// no prime numbers < 2
    return 0;
  }
  let sumPrime =2; // intialize sum to value 2 

 //starting loop at 3 ,
 //increment by 2 to skip evens up to num
 for(let i =3;i<=num ; i+=2){  
   let divisors = 0; 
// check for divisors < square root of number 
   for(let j = 2; j<=Math.sqrt(i);j++){
    if(i%j == 0 ){
      divisors++;
       }  
     }
    if(!divisors){
      sumPrime += i;} // add prime number to sum
   } 
   console.timeEnd('better?');
  return sumPrime;
}

I’d still break out of the loop here and then you save some comparisons.

1 Like

Interesting topic. As a beginner in developing, I am constantly worried about script speed. Although as proved today, i still cant get out of my own way bogging things down.

Everyone wants to write fast code. Personally, I recommend people write clear code and then work on making the slowest parts fast.

1 Like

Ok, thanks. I see, no need to keep going on the larger numbers. I was thinking on the small scale i guess. I will update my solution now for my reference. Edit - I found the spoiler tag. LOL

1 Like

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.