 # 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?

``````
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
console.log(arr)
}
}
return arr

}

sumPrimes(977);

``````

Challenge: Sum All Primes

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.

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

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

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.