Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

Readability

#Readability FirstFirst, long lines make your code hard to read. The easier the code is to read, the easier it is for you to see what it does and spot potential improvements. It's also easier for others to suggest improvements to your code if it's easy for them to read it.

It is true that sometimes a one line expression can run faster than a loop over several lines, but even a one liner can be displayed over several lines by using brackets or parentheses for implicit line continuation. This still counts as a one liner from the bytecode compiler's point of view and will run just as fast. I won't point out minor spacing issues as you use spacing very readably in some parts, so I'm guessing the other parts are not lack of knowledge. There's PEP 8 if you want finer guidance on presentation, but I'm reviewing for time performance here.

Comments

#Comments II heard Eratosthenes died, no need to worry about copyright - you can use his full name in the comments. More seriously, comments are discarded before the code is compiled to bytecode and run, so you don't need to worry about shortening them. Longer comments make the file slightly larger, but they do not affect the running time at all. Full words and clear sentences are good.

Tweaks

#Tweaks SmallSmall improvements to speed can be made by being aware of how Python works. For example, defining r = xrange(2, int(end ** 2) + 2) before your loop and using r inside your loop will save Python having to define the range each time through the loop (including calculating end ** 2 each time through the loop). Such fine tuning won't make much difference if you have an inefficient algorithm though.

Algorithm

#Algorithm TheThe biggest improvements in speed are very often from changing the algorithm - using a different approach rather than just tweaking the performance of the current approach There are parts of your long line (sieve of Eratosthenes algorithm) that appear to be making unnecessary extra work:

for p in xrange(2, int(end ** 0.5) + 2)

This gives you the range up to and including int(end ** 0.5) + 1), which is always more than the square root, even when considering a square number. Since you only need to check numbers which are less than or equal to the square root, I would change the + 2 to say + 1.

The major inefficiency with your algorithm is that you are checking all numbers up to the square root, rather than all prime numbers up to the square root. To see why this is unnecessary, consider that when you remove all multiples of 4, they have already been removed when you removed all multiples of 2. Then the same when you remove all multiples of 6, all multiples of 8, and so on. The same thing happens again with multiples of 6, 9, 12 because you have already removed all multiples of 3. With large numbers this will cause a huge amount of repetition - creating and taking differences of sets which already have no elements in common, and repeating this unnecessary step huge numbers of times.

For this reason it is well worth your time to think about how you could only remove multiples of prime numbers. The extra time your program takes keeping a list of prime numbers already found will save you so much time over the course of the run that you will see a very significant speed up for large numbers.

Josay's answer already shows such an improvement in the example code.

#Readability First, long lines make your code hard to read. The easier the code is to read, the easier it is for you to see what it does and spot potential improvements. It's also easier for others to suggest improvements to your code if it's easy for them to read it.

It is true that sometimes a one line expression can run faster than a loop over several lines, but even a one liner can be displayed over several lines by using brackets or parentheses for implicit line continuation. This still counts as a one liner from the bytecode compiler's point of view and will run just as fast. I won't point out minor spacing issues as you use spacing very readably in some parts, so I'm guessing the other parts are not lack of knowledge. There's PEP 8 if you want finer guidance on presentation, but I'm reviewing for time performance here.

#Comments I heard Eratosthenes died, no need to worry about copyright - you can use his full name in the comments. More seriously, comments are discarded before the code is compiled to bytecode and run, so you don't need to worry about shortening them. Longer comments make the file slightly larger, but they do not affect the running time at all. Full words and clear sentences are good.

#Tweaks Small improvements to speed can be made by being aware of how Python works. For example, defining r = xrange(2, int(end ** 2) + 2) before your loop and using r inside your loop will save Python having to define the range each time through the loop (including calculating end ** 2 each time through the loop). Such fine tuning won't make much difference if you have an inefficient algorithm though.

#Algorithm The biggest improvements in speed are very often from changing the algorithm - using a different approach rather than just tweaking the performance of the current approach There are parts of your long line (sieve of Eratosthenes algorithm) that appear to be making unnecessary extra work:

for p in xrange(2, int(end ** 0.5) + 2)

This gives you the range up to and including int(end ** 0.5) + 1), which is always more than the square root, even when considering a square number. Since you only need to check numbers which are less than or equal to the square root, I would change the + 2 to say + 1.

The major inefficiency with your algorithm is that you are checking all numbers up to the square root, rather than all prime numbers up to the square root. To see why this is unnecessary, consider that when you remove all multiples of 4, they have already been removed when you removed all multiples of 2. Then the same when you remove all multiples of 6, all multiples of 8, and so on. The same thing happens again with multiples of 6, 9, 12 because you have already removed all multiples of 3. With large numbers this will cause a huge amount of repetition - creating and taking differences of sets which already have no elements in common, and repeating this unnecessary step huge numbers of times.

For this reason it is well worth your time to think about how you could only remove multiples of prime numbers. The extra time your program takes keeping a list of prime numbers already found will save you so much time over the course of the run that you will see a very significant speed up for large numbers.

Josay's answer already shows such an improvement in the example code.

Readability

First, long lines make your code hard to read. The easier the code is to read, the easier it is for you to see what it does and spot potential improvements. It's also easier for others to suggest improvements to your code if it's easy for them to read it.

It is true that sometimes a one line expression can run faster than a loop over several lines, but even a one liner can be displayed over several lines by using brackets or parentheses for implicit line continuation. This still counts as a one liner from the bytecode compiler's point of view and will run just as fast. I won't point out minor spacing issues as you use spacing very readably in some parts, so I'm guessing the other parts are not lack of knowledge. There's PEP 8 if you want finer guidance on presentation, but I'm reviewing for time performance here.

Comments

I heard Eratosthenes died, no need to worry about copyright - you can use his full name in the comments. More seriously, comments are discarded before the code is compiled to bytecode and run, so you don't need to worry about shortening them. Longer comments make the file slightly larger, but they do not affect the running time at all. Full words and clear sentences are good.

Tweaks

Small improvements to speed can be made by being aware of how Python works. For example, defining r = xrange(2, int(end ** 2) + 2) before your loop and using r inside your loop will save Python having to define the range each time through the loop (including calculating end ** 2 each time through the loop). Such fine tuning won't make much difference if you have an inefficient algorithm though.

Algorithm

The biggest improvements in speed are very often from changing the algorithm - using a different approach rather than just tweaking the performance of the current approach There are parts of your long line (sieve of Eratosthenes algorithm) that appear to be making unnecessary extra work:

for p in xrange(2, int(end ** 0.5) + 2)

This gives you the range up to and including int(end ** 0.5) + 1), which is always more than the square root, even when considering a square number. Since you only need to check numbers which are less than or equal to the square root, I would change the + 2 to say + 1.

The major inefficiency with your algorithm is that you are checking all numbers up to the square root, rather than all prime numbers up to the square root. To see why this is unnecessary, consider that when you remove all multiples of 4, they have already been removed when you removed all multiples of 2. Then the same when you remove all multiples of 6, all multiples of 8, and so on. The same thing happens again with multiples of 6, 9, 12 because you have already removed all multiples of 3. With large numbers this will cause a huge amount of repetition - creating and taking differences of sets which already have no elements in common, and repeating this unnecessary step huge numbers of times.

For this reason it is well worth your time to think about how you could only remove multiples of prime numbers. The extra time your program takes keeping a list of prime numbers already found will save you so much time over the course of the run that you will see a very significant speed up for large numbers.

Josay's answer already shows such an improvement in the example code.

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

#Readability First, long lines make your code hard to read. The easier the code is to read, the easier it is for you to see what it does and spot potential improvements. It's also easier for others to suggest improvements to your code if it's easy for them to read it.

It is true that sometimes a one line expression can run faster than a loop over several lines, but even a one liner can be displayed over several lines by using brackets or parentheses for implicit line continuation. This still counts as a one liner from the bytecode compiler's point of view and will run just as fast. I won't point out minor spacing issues as you use spacing very readably in some parts, so I'm guessing the other parts are not lack of knowledge. There's PEP 8 if you want finer guidance on presentation, but I'm reviewing for time performance here.

#Comments I heard Eratosthenes died, no need to worry about copyright - you can use his full name in the comments. More seriously, comments are discarded before the code is compiled to bytecode and run, so you don't need to worry about shortening them. Longer comments make the file slightly larger, but they do not affect the running time at all. Full words and clear sentences are good.

#Tweaks Small improvements to speed can be made by being aware of how Python works. For example, defining r = xrange(2, int(end ** 2) + 2) before your loop and using r inside your loop will save Python having to define the range each time through the loop (including calculating end ** 2 each time through the loop). Such fine tuning won't make much difference if you have an inefficient algorithm though.

#Algorithm The biggest improvements in speed are very often from changing the algorithm - using a different approach rather than just tweaking the performance of the current approach There are parts of your long line (sieve of Eratosthenes algorithm) that appear to be making unnecessary extra work:

for p in xrange(2, int(end ** 0.5) + 2)

This gives you the range up to and including int(end ** 0.5) + 1), which is always more than the square root, even when considering a square number. Since you only need to check numbers which are less than or equal to the square root, I would change the + 2 to say + 1.

The major inefficiency with your algorithm is that you are checking all numbers up to the square root, rather than all prime numbers up to the square root. To see why this is unnecessary, consider that when you remove all multiples of 4, they have already been removed when you removed all multiples of 2. Then the same when you remove all multiples of 6, all multiples of 8, and so on. The same thing happens again with multiples of 6, 9, 12 because you have already removed all multiples of 3. With large numbers this will cause a huge amount of repetition - creating and taking differences of sets which already have no elements in common, and repeating this unnecessary step huge numbers of times.

For this reason it is well worth your time to think about how you could only remove multiples of prime numbers. The extra time your program takes keeping a list of prime numbers already found will save you so much time over the course of the run that you will see a very significant speed up for large numbers.

Josay's Josay's answer already shows such an improvement in the example code.

#Readability First, long lines make your code hard to read. The easier the code is to read, the easier it is for you to see what it does and spot potential improvements. It's also easier for others to suggest improvements to your code if it's easy for them to read it.

It is true that sometimes a one line expression can run faster than a loop over several lines, but even a one liner can be displayed over several lines by using brackets or parentheses for implicit line continuation. This still counts as a one liner from the bytecode compiler's point of view and will run just as fast. I won't point out minor spacing issues as you use spacing very readably in some parts, so I'm guessing the other parts are not lack of knowledge. There's PEP 8 if you want finer guidance on presentation, but I'm reviewing for time performance here.

#Comments I heard Eratosthenes died, no need to worry about copyright - you can use his full name in the comments. More seriously, comments are discarded before the code is compiled to bytecode and run, so you don't need to worry about shortening them. Longer comments make the file slightly larger, but they do not affect the running time at all. Full words and clear sentences are good.

#Tweaks Small improvements to speed can be made by being aware of how Python works. For example, defining r = xrange(2, int(end ** 2) + 2) before your loop and using r inside your loop will save Python having to define the range each time through the loop (including calculating end ** 2 each time through the loop). Such fine tuning won't make much difference if you have an inefficient algorithm though.

#Algorithm The biggest improvements in speed are very often from changing the algorithm - using a different approach rather than just tweaking the performance of the current approach There are parts of your long line (sieve of Eratosthenes algorithm) that appear to be making unnecessary extra work:

for p in xrange(2, int(end ** 0.5) + 2)

This gives you the range up to and including int(end ** 0.5) + 1), which is always more than the square root, even when considering a square number. Since you only need to check numbers which are less than or equal to the square root, I would change the + 2 to say + 1.

The major inefficiency with your algorithm is that you are checking all numbers up to the square root, rather than all prime numbers up to the square root. To see why this is unnecessary, consider that when you remove all multiples of 4, they have already been removed when you removed all multiples of 2. Then the same when you remove all multiples of 6, all multiples of 8, and so on. The same thing happens again with multiples of 6, 9, 12 because you have already removed all multiples of 3. With large numbers this will cause a huge amount of repetition - creating and taking differences of sets which already have no elements in common, and repeating this unnecessary step huge numbers of times.

For this reason it is well worth your time to think about how you could only remove multiples of prime numbers. The extra time your program takes keeping a list of prime numbers already found will save you so much time over the course of the run that you will see a very significant speed up for large numbers.

Josay's answer already shows such an improvement in the example code.

#Readability First, long lines make your code hard to read. The easier the code is to read, the easier it is for you to see what it does and spot potential improvements. It's also easier for others to suggest improvements to your code if it's easy for them to read it.

It is true that sometimes a one line expression can run faster than a loop over several lines, but even a one liner can be displayed over several lines by using brackets or parentheses for implicit line continuation. This still counts as a one liner from the bytecode compiler's point of view and will run just as fast. I won't point out minor spacing issues as you use spacing very readably in some parts, so I'm guessing the other parts are not lack of knowledge. There's PEP 8 if you want finer guidance on presentation, but I'm reviewing for time performance here.

#Comments I heard Eratosthenes died, no need to worry about copyright - you can use his full name in the comments. More seriously, comments are discarded before the code is compiled to bytecode and run, so you don't need to worry about shortening them. Longer comments make the file slightly larger, but they do not affect the running time at all. Full words and clear sentences are good.

#Tweaks Small improvements to speed can be made by being aware of how Python works. For example, defining r = xrange(2, int(end ** 2) + 2) before your loop and using r inside your loop will save Python having to define the range each time through the loop (including calculating end ** 2 each time through the loop). Such fine tuning won't make much difference if you have an inefficient algorithm though.

#Algorithm The biggest improvements in speed are very often from changing the algorithm - using a different approach rather than just tweaking the performance of the current approach There are parts of your long line (sieve of Eratosthenes algorithm) that appear to be making unnecessary extra work:

for p in xrange(2, int(end ** 0.5) + 2)

This gives you the range up to and including int(end ** 0.5) + 1), which is always more than the square root, even when considering a square number. Since you only need to check numbers which are less than or equal to the square root, I would change the + 2 to say + 1.

The major inefficiency with your algorithm is that you are checking all numbers up to the square root, rather than all prime numbers up to the square root. To see why this is unnecessary, consider that when you remove all multiples of 4, they have already been removed when you removed all multiples of 2. Then the same when you remove all multiples of 6, all multiples of 8, and so on. The same thing happens again with multiples of 6, 9, 12 because you have already removed all multiples of 3. With large numbers this will cause a huge amount of repetition - creating and taking differences of sets which already have no elements in common, and repeating this unnecessary step huge numbers of times.

For this reason it is well worth your time to think about how you could only remove multiples of prime numbers. The extra time your program takes keeping a list of prime numbers already found will save you so much time over the course of the run that you will see a very significant speed up for large numbers.

Josay's answer already shows such an improvement in the example code.

Source Link

#Readability First, long lines make your code hard to read. The easier the code is to read, the easier it is for you to see what it does and spot potential improvements. It's also easier for others to suggest improvements to your code if it's easy for them to read it.

It is true that sometimes a one line expression can run faster than a loop over several lines, but even a one liner can be displayed over several lines by using brackets or parentheses for implicit line continuation. This still counts as a one liner from the bytecode compiler's point of view and will run just as fast. I won't point out minor spacing issues as you use spacing very readably in some parts, so I'm guessing the other parts are not lack of knowledge. There's PEP 8 if you want finer guidance on presentation, but I'm reviewing for time performance here.

#Comments I heard Eratosthenes died, no need to worry about copyright - you can use his full name in the comments. More seriously, comments are discarded before the code is compiled to bytecode and run, so you don't need to worry about shortening them. Longer comments make the file slightly larger, but they do not affect the running time at all. Full words and clear sentences are good.

#Tweaks Small improvements to speed can be made by being aware of how Python works. For example, defining r = xrange(2, int(end ** 2) + 2) before your loop and using r inside your loop will save Python having to define the range each time through the loop (including calculating end ** 2 each time through the loop). Such fine tuning won't make much difference if you have an inefficient algorithm though.

#Algorithm The biggest improvements in speed are very often from changing the algorithm - using a different approach rather than just tweaking the performance of the current approach There are parts of your long line (sieve of Eratosthenes algorithm) that appear to be making unnecessary extra work:

for p in xrange(2, int(end ** 0.5) + 2)

This gives you the range up to and including int(end ** 0.5) + 1), which is always more than the square root, even when considering a square number. Since you only need to check numbers which are less than or equal to the square root, I would change the + 2 to say + 1.

The major inefficiency with your algorithm is that you are checking all numbers up to the square root, rather than all prime numbers up to the square root. To see why this is unnecessary, consider that when you remove all multiples of 4, they have already been removed when you removed all multiples of 2. Then the same when you remove all multiples of 6, all multiples of 8, and so on. The same thing happens again with multiples of 6, 9, 12 because you have already removed all multiples of 3. With large numbers this will cause a huge amount of repetition - creating and taking differences of sets which already have no elements in common, and repeating this unnecessary step huge numbers of times.

For this reason it is well worth your time to think about how you could only remove multiples of prime numbers. The extra time your program takes keeping a list of prime numbers already found will save you so much time over the course of the run that you will see a very significant speed up for large numbers.

Josay's answer already shows such an improvement in the example code.

lang-py

AltStyle によって変換されたページ (->オリジナル) /