#Comments are distracting#
Comments are distracting
The number of comments in the code is too much and distracts from reading the code. I would prefer one large comment before each function explaining things rather than one comment per line.
#Inefficiencies#
Inefficiencies
There are some places in the code that are inefficient:
When you find a prime factor, you set
i = 1
. You don't need to do that because you won't find any more prime factors less thani
. What you do need to do is keep retryingi
because the same prime factor can divide multiple times into your number.After each division, you check if the remaining
number
is a prime. I'm not sure if that makes things faster or slower, but ifnumber
is prime, you should return immediately instead of continuing in the loop, because there will be no more divisors.
#Faster method#
Faster method
A faster way of doing this would be to precompute the primes from 1..N using something like a Sieve of Eratosthenes algorithm. That way, your isPrime()
function becomes an array lookup instead of a loop. Also, if you generate a list of primes from the Sieve, your main factoring loop will be much faster because you can only divide by primes instead of dividing by every number.
I wrote a program using this method and it was 50x faster than your program when computing up to 100000, and 600x faster than your program when computing up to 1000000 (0.60 seconds vs 365 seconds).
#Comments are distracting#
The number of comments in the code is too much and distracts from reading the code. I would prefer one large comment before each function explaining things rather than one comment per line.
#Inefficiencies#
There are some places in the code that are inefficient:
When you find a prime factor, you set
i = 1
. You don't need to do that because you won't find any more prime factors less thani
. What you do need to do is keep retryingi
because the same prime factor can divide multiple times into your number.After each division, you check if the remaining
number
is a prime. I'm not sure if that makes things faster or slower, but ifnumber
is prime, you should return immediately instead of continuing in the loop, because there will be no more divisors.
#Faster method#
A faster way of doing this would be to precompute the primes from 1..N using something like a Sieve of Eratosthenes algorithm. That way, your isPrime()
function becomes an array lookup instead of a loop. Also, if you generate a list of primes from the Sieve, your main factoring loop will be much faster because you can only divide by primes instead of dividing by every number.
I wrote a program using this method and it was 50x faster than your program when computing up to 100000, and 600x faster than your program when computing up to 1000000 (0.60 seconds vs 365 seconds).
Comments are distracting
The number of comments in the code is too much and distracts from reading the code. I would prefer one large comment before each function explaining things rather than one comment per line.
Inefficiencies
There are some places in the code that are inefficient:
When you find a prime factor, you set
i = 1
. You don't need to do that because you won't find any more prime factors less thani
. What you do need to do is keep retryingi
because the same prime factor can divide multiple times into your number.After each division, you check if the remaining
number
is a prime. I'm not sure if that makes things faster or slower, but ifnumber
is prime, you should return immediately instead of continuing in the loop, because there will be no more divisors.
Faster method
A faster way of doing this would be to precompute the primes from 1..N using something like a Sieve of Eratosthenes algorithm. That way, your isPrime()
function becomes an array lookup instead of a loop. Also, if you generate a list of primes from the Sieve, your main factoring loop will be much faster because you can only divide by primes instead of dividing by every number.
I wrote a program using this method and it was 50x faster than your program when computing up to 100000, and 600x faster than your program when computing up to 1000000 (0.60 seconds vs 365 seconds).
#Comments are distracting#
The number of comments in the code is too much and distracts from reading the code. I would prefer one large comment before each function explaining things rather than one comment per line.
#Inefficiencies#
There are some places in the code that are inefficient:
When you find a prime factor, you set
i = 1
. You don't need to do that because you won't find any more prime factors less thani
. What you do need to do is keep retryingi
because the same prime factor can divide multiple times into your number.After each division, you check if the remaining
number
is a prime. I'm not sure if that makes things faster or slower, but ifnumber
is prime, you should return immediately instead of continuing in the loop, because there will be no more divisors.
#Faster method#
A faster way of doing this would be to precompute the primes from 1..N using something like a Sieve of Eratosthenes algorithm. That way, your isPrime()
function becomes an array lookup instead of a loop. Your Also, if you generate a list of primes from the Sieve, your main factoring loop will also benefitbe much faster because you can only try dividingdivide by primes instead of dividing by every number.
I wrote a program using this method and it was about 7x50x faster than your program when computing up to 100000, and 600x faster than your program when computing up to 1000000 (0.60 seconds vs 365 seconds).
#Comments are distracting#
The number of comments in the code is too much and distracts from reading the code. I would prefer one large comment before each function explaining things rather than one comment per line.
#Inefficiencies#
There are some places in the code that are inefficient:
When you find a prime factor, you set
i = 1
. You don't need to do that because you won't find any more prime factors less thani
. What you do need to do is keep retryingi
because the same prime factor can divide multiple times into your number.After each division, you check if the remaining
number
is a prime. I'm not sure if that makes things faster or slower, but ifnumber
is prime, you should return immediately instead of continuing in the loop, because there will be no more divisors.
#Faster method#
A faster way of doing this would be to precompute the primes from 1..N using something like a Sieve of Eratosthenes algorithm. That way, your isPrime()
function becomes an array lookup instead of a loop. Your main factoring loop will also benefit because you can only try dividing by primes instead of by every number.
I wrote a program using this method and it was about 7x faster than your program when computing up to 100000.
#Comments are distracting#
The number of comments in the code is too much and distracts from reading the code. I would prefer one large comment before each function explaining things rather than one comment per line.
#Inefficiencies#
There are some places in the code that are inefficient:
When you find a prime factor, you set
i = 1
. You don't need to do that because you won't find any more prime factors less thani
. What you do need to do is keep retryingi
because the same prime factor can divide multiple times into your number.After each division, you check if the remaining
number
is a prime. I'm not sure if that makes things faster or slower, but ifnumber
is prime, you should return immediately instead of continuing in the loop, because there will be no more divisors.
#Faster method#
A faster way of doing this would be to precompute the primes from 1..N using something like a Sieve of Eratosthenes algorithm. That way, your isPrime()
function becomes an array lookup instead of a loop. Also, if you generate a list of primes from the Sieve, your main factoring loop will be much faster because you can only divide by primes instead of dividing by every number.
I wrote a program using this method and it was 50x faster than your program when computing up to 100000, and 600x faster than your program when computing up to 1000000 (0.60 seconds vs 365 seconds).
#Comments are distracting#
The number of comments in the code is too much and distracts from reading the code. I would prefer one large comment before each function explaining things rather than one comment per line.
#Inefficiencies#
There are some places in the code that are inefficient:
When you find a prime factor, you set
i = 1
. You don't need to do that because you won't find any more prime factors less thani
. What you do need to do is keep retryingi
because the same prime factor can divide multiple times into your number.After each division, you check if the remaining
number
is a prime. I'm not sure if that makes things faster or slower, but ifnumber
is prime, you should return immediately instead of continuing in the loop, because there will be no more divisors.
#Faster method#
A faster way of doing this would be to precompute the primes from 1..N using something like a Sieve of Eratosthenes algorithm. That way, your isPrime()
function becomes an array lookup instead of a loop. Your main factoring loop will also benefit because you can only try dividing by primes instead of by every number.
I wrote a program using this method and it was about 7x faster than your program when computing up to 100000.