Is this a good code?

Tell us what’s happening:
I think it’s not good enough since i checked solutions and they are hard for me to understand. Is this okay to write the solutions the way i did?

Your code so far


function sumPrimes(num) {
let arr = 0
for (let i = 2; i<=num; i++) {
  let check = []
  for (let j = 2; j<=i; j++) {
    if (i % j == 0) {
      check.push(i)
    }
  }
  if (check.length == 1) {
    arr = arr + check[0]
    console.log(arr)
  }
}
return arr


}

sumPrimes(977);

Your browser information:

Challenge: Sum All Primes

Link to the challenge:

One thing missing is an early break. If you’re only doing something with check array if its length is 1, do you really need to keep adding more items to it?

Hi and welcome to the forum!

It is ok if your code doesn’t look as polished as the sample solutions. The first measure is if your code works, the second is if your code is readable, and the third measure is if your code is efficient.

I do think that you can improve your solution however.

First, your sum variable is called arr but is not an array.

You know a number is not prime if you find a divisor that is not the number itself. Why keep looking for divisors? That’s more work than you need to do.

Even numbers other than 2 can’t be prime, so why check even i?

If 2 is not a divisor, then you don’t need to check if other even j are divisors. Though you don’t need to check for even divisors if you aren’t checking the primality of even numbers.

You don’t need to check all the way to i for divisors. The largest possible prime divisor of a number is much smaller.

I hope these hints help you improve your code.

You could get an even more efficient solution if you look at a Seive of Eratoshenes based approach like I discussed here: JavaScript Performance Measurement & Variability

2 Likes

Thank you for your kind reply!

Your hints are brilliant. Going to post here my new better solution as sonn as i write it according to your hints! I Really appreciate it.

1 Like
function sumPrimes(num) {
  let result = 0
  for (let i = 2; i<=num; i++) {
    let check = []
    for (let j = 2; j<=31; j++) {
        if (i != j) {
          if (i % j == 0) {
            check.push(i)
          }
        }
    }
    if (check.length == 0) {
      result += i
    }
  }
  return result
} 
 

sumPrimes(977);

i guess it is a little bit better

forgot to mention your login

Nice improvements!

I would adjust this a little bit. You only need to check 2, 3, 5, 7, 9, 11, 13, 15…
because 2 is the only even prime number.
That’ll cut down how much work you do by 1/2.

I’m not sure why you stop at the hard-coded number 31? I would use the square root function to set this bound. You also only need to check if the number is divisible by 2, 3, 5, 7, 9, 11…
That’ll cut down how much work you do by 1/2 again.

If you use the square root function, then you won’t have to check if i == j here.

Also, you can avoid making the array check altogether. You only need a boolean variable that is true if you have not found a divisor and false when you have.

You can break out of the j loop when you find a divisor to make your code a little bit faster yet.