Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

I'll firstly talk about micro-optimizations then go for major optimizations that can be done in your code.

Don't use the while loop when you can instead use for i in xrange(). It is very slow. Take a look here and here here. There are some limitations butxrange unless you are hitting that limit your code would get faster.

The inner while loop's conditions can be improved. As you have placed calculating math.sqrt() in the conditions it is calculated every time. That makes it very slow because finding square root consumes much time. You should use a variable before the loop to store that value. The second condition is not needed. Instead of checking always for the value of prime you can delete this condition as well as the variable and simply use a break in the if statement.

About using append. You are appending numbers and popping them if they are not primes. Not needed. Just append at the correct condition and use break

Also read the Python performance tips link given in Python Tags' wiki on codereview. I would have written it like this:

import math
def primeList(listLength):
 if listLength < 1:
 return []
 plist = [2]
 j = 3
 sqr_root = math.sqrt
 list_app = plist.append
 while listLength > len(plist):
 temp = sqr_root(j)
 for i in xrange(len(plist)):
 if j % plist[i] == 0:
 break
 if plist[i] > temp:
 list_app(j)
 break
 
 j += 2
 return plist

Notice that I defined math.sqrt as something. You'll find that in the Python Performance tips. Also your implementation had a bug. If I entered anything less than 2 it returned [2, 3] which was incorrect result.

This worked in 44 % time that your original function took. Your code's timing and my code's timing. Note that memory usage is lower in my case. I changed my code a bit. This new code uses only 34% time of OP's code.

Now done with micro-optimizations I'll get to major optimizations.

Using this approach to find the list of prime numbers is actually very naive. I also used it in the beginning and after much headache and waiting for outputs to come I found that such approaches can be very slow. Nothing you change in Python's syntax can offset the advantage of using a better algorithm. Check out this. It is not to difficult to implement. You can look at my github to get a basic implementation. You'll have to tweak it but you won't have to write it from the beginning.

Hope this helped.

I'll firstly talk about micro-optimizations then go for major optimizations that can be done in your code.

Don't use the while loop when you can instead use for i in xrange(). It is very slow. Take a look here and here. There are some limitations butxrange unless you are hitting that limit your code would get faster.

The inner while loop's conditions can be improved. As you have placed calculating math.sqrt() in the conditions it is calculated every time. That makes it very slow because finding square root consumes much time. You should use a variable before the loop to store that value. The second condition is not needed. Instead of checking always for the value of prime you can delete this condition as well as the variable and simply use a break in the if statement.

About using append. You are appending numbers and popping them if they are not primes. Not needed. Just append at the correct condition and use break

Also read the Python performance tips link given in Python Tags' wiki on codereview. I would have written it like this:

import math
def primeList(listLength):
 if listLength < 1:
 return []
 plist = [2]
 j = 3
 sqr_root = math.sqrt
 list_app = plist.append
 while listLength > len(plist):
 temp = sqr_root(j)
 for i in xrange(len(plist)):
 if j % plist[i] == 0:
 break
 if plist[i] > temp:
 list_app(j)
 break
 
 j += 2
 return plist

Notice that I defined math.sqrt as something. You'll find that in the Python Performance tips. Also your implementation had a bug. If I entered anything less than 2 it returned [2, 3] which was incorrect result.

This worked in 44 % time that your original function took. Your code's timing and my code's timing. Note that memory usage is lower in my case. I changed my code a bit. This new code uses only 34% time of OP's code.

Now done with micro-optimizations I'll get to major optimizations.

Using this approach to find the list of prime numbers is actually very naive. I also used it in the beginning and after much headache and waiting for outputs to come I found that such approaches can be very slow. Nothing you change in Python's syntax can offset the advantage of using a better algorithm. Check out this. It is not to difficult to implement. You can look at my github to get a basic implementation. You'll have to tweak it but you won't have to write it from the beginning.

Hope this helped.

I'll firstly talk about micro-optimizations then go for major optimizations that can be done in your code.

Don't use the while loop when you can instead use for i in xrange(). It is very slow. Take a look here and here. There are some limitations butxrange unless you are hitting that limit your code would get faster.

The inner while loop's conditions can be improved. As you have placed calculating math.sqrt() in the conditions it is calculated every time. That makes it very slow because finding square root consumes much time. You should use a variable before the loop to store that value. The second condition is not needed. Instead of checking always for the value of prime you can delete this condition as well as the variable and simply use a break in the if statement.

About using append. You are appending numbers and popping them if they are not primes. Not needed. Just append at the correct condition and use break

Also read the Python performance tips link given in Python Tags' wiki on codereview. I would have written it like this:

import math
def primeList(listLength):
 if listLength < 1:
 return []
 plist = [2]
 j = 3
 sqr_root = math.sqrt
 list_app = plist.append
 while listLength > len(plist):
 temp = sqr_root(j)
 for i in xrange(len(plist)):
 if j % plist[i] == 0:
 break
 if plist[i] > temp:
 list_app(j)
 break
 
 j += 2
 return plist

Notice that I defined math.sqrt as something. You'll find that in the Python Performance tips. Also your implementation had a bug. If I entered anything less than 2 it returned [2, 3] which was incorrect result.

This worked in 44 % time that your original function took. Your code's timing and my code's timing. Note that memory usage is lower in my case. I changed my code a bit. This new code uses only 34% time of OP's code.

Now done with micro-optimizations I'll get to major optimizations.

Using this approach to find the list of prime numbers is actually very naive. I also used it in the beginning and after much headache and waiting for outputs to come I found that such approaches can be very slow. Nothing you change in Python's syntax can offset the advantage of using a better algorithm. Check out this. It is not to difficult to implement. You can look at my github to get a basic implementation. You'll have to tweak it but you won't have to write it from the beginning.

Hope this helped.

added 105 characters in body
Source Link
Aseem Bansal
  • 2.3k
  • 3
  • 22
  • 37

I'll firstly talk about micro-optimizations then go for major optimizations that can be done in your code.

Don't use the while loop when you can instead use for i in xrange(). It is very slow. Take a look here and here. There are some limitations butxrange unless you are hitting that limit your code would get faster.

The inner while loop's conditions can be improved. As you have placed calculating math.sqrt() in the conditions it is calculated every time. That makes it very slow because finding square root consumes much time. You should use a variable before the loop to store that value. The second condition is not needed. Instead of checking always for the value of prime you can delete this condition as well as the variable and simply use a break in the if statement.

About using append. You are appending numbers and popping them if they are not primes. Not needed. Just append at the correct condition and use break

Also read the Python performance tips link given in Python Tags' wiki on codereview. I would have written it like this:

import math
def primeList(listLength):
 if listLength < 1:
 return []
 plist = [2]
 j = 3
 sqr_root = math.sqrt
 list_app = plist.append
 while listLength > len(plist):
 temp = sqr_root(j)
 for i in xrange(len(plist)):
 if j % plist[i] == 0:
 break
 if plist[i] > temp:
 list_app(j)
 break
 
 j += 2
 return plist

Notice that I defined math.sqrt as something. You'll find that in the Python Performance tips. Also your implementation had a bug. If I entered anything less than 2 it returned [2, 3] which was incorrect result.

This worked in 44 % time that your original function took. Your code's timing and my code's timing. Note that memory usage is lower in my case. I changed my code a bit. This new code uses only 34% time of OP's code.

Now done with micro-optimizations I'll get to major optimizations.

Using this approach to find the list of prime numbers is actually very naive. I also used it in the beginning and after much headache and waiting for outputs to come I found that such approaches can be very slow. Nothing you change in Python's syntax can offset the advantage of using a better algorithm. Check out this. It is not to difficult to implement. You can look at my github to get a basic implementation. You'll have to tweak it but you won't have to write it from the beginning.

Hope this helped.

I'll firstly talk about micro-optimizations then go for major optimizations that can be done in your code.

Don't use the while loop when you can instead use for i in xrange(). It is very slow. Take a look here and here. There are some limitations butxrange unless you are hitting that limit your code would get faster.

The inner while loop's conditions can be improved. As you have placed calculating math.sqrt() in the conditions it is calculated every time. That makes it very slow because finding square root consumes much time. You should use a variable before the loop to store that value. The second condition is not needed. Instead of checking always for the value of prime you can delete this condition as well as the variable and simply use a break in the if statement.

About using append. You are appending numbers and popping them if they are not primes. Not needed. Just append at the correct condition and use break

Also read the Python performance tips link given in Python Tags' wiki on codereview. I would have written it like this:

import math
def primeList(listLength):
 if listLength < 1:
 return []
 plist = [2]
 j = 3
 sqr_root = math.sqrt
 list_app = plist.append
 while listLength > len(plist):
 temp = sqr_root(j)
 for i in xrange(len(plist)):
 if j % plist[i] == 0:
 break
 if plist[i] > temp:
 list_app(j)
 break
 
 j += 2
 return plist

Notice that I defined math.sqrt as something. You'll find that in the Python Performance tips. Also your implementation had a bug. If I entered anything less than 2 it returned [2, 3] which was incorrect result.

This worked in 44 % time that your original function took. Your code's timing and my code's timing. Note that memory usage is lower in my case.

Now done with micro-optimizations I'll get to major optimizations.

Using this approach to find the list of prime numbers is actually very naive. I also used it in the beginning and after much headache and waiting for outputs to come I found that such approaches can be very slow. Nothing you change in Python's syntax can offset the advantage of using a better algorithm. Check out this. It is not to difficult to implement. You can look at my github to get a basic implementation. You'll have to tweak it but you won't have to write it from the beginning.

Hope this helped.

I'll firstly talk about micro-optimizations then go for major optimizations that can be done in your code.

Don't use the while loop when you can instead use for i in xrange(). It is very slow. Take a look here and here. There are some limitations butxrange unless you are hitting that limit your code would get faster.

The inner while loop's conditions can be improved. As you have placed calculating math.sqrt() in the conditions it is calculated every time. That makes it very slow because finding square root consumes much time. You should use a variable before the loop to store that value. The second condition is not needed. Instead of checking always for the value of prime you can delete this condition as well as the variable and simply use a break in the if statement.

About using append. You are appending numbers and popping them if they are not primes. Not needed. Just append at the correct condition and use break

Also read the Python performance tips link given in Python Tags' wiki on codereview. I would have written it like this:

import math
def primeList(listLength):
 if listLength < 1:
 return []
 plist = [2]
 j = 3
 sqr_root = math.sqrt
 list_app = plist.append
 while listLength > len(plist):
 temp = sqr_root(j)
 for i in xrange(len(plist)):
 if j % plist[i] == 0:
 break
 if plist[i] > temp:
 list_app(j)
 break
 
 j += 2
 return plist

Notice that I defined math.sqrt as something. You'll find that in the Python Performance tips. Also your implementation had a bug. If I entered anything less than 2 it returned [2, 3] which was incorrect result.

This worked in 44 % time that your original function took. Your code's timing and my code's timing. Note that memory usage is lower in my case. I changed my code a bit. This new code uses only 34% time of OP's code.

Now done with micro-optimizations I'll get to major optimizations.

Using this approach to find the list of prime numbers is actually very naive. I also used it in the beginning and after much headache and waiting for outputs to come I found that such approaches can be very slow. Nothing you change in Python's syntax can offset the advantage of using a better algorithm. Check out this. It is not to difficult to implement. You can look at my github to get a basic implementation. You'll have to tweak it but you won't have to write it from the beginning.

Hope this helped.

added 223 characters in body
Source Link
Aseem Bansal
  • 2.3k
  • 3
  • 22
  • 37

I'll firstly talk about micro-optimizations then go for major optimizations that can be done in your code.

Don't use the while loop when you can instead use for i in xrange(). It is very slow. Take a look here and here. There are some limitations butxrange unless you are hitting that limit your code would get faster.

The inner while loop's conditions can be improved. As you have placed calculating math.sqrt() in the conditions it is calculated every time. That makes it very slow because finding square root consumes much time. You should use a variable before the loop to store that value. The second condition is not needed. Instead of checking always for the value of prime you can delete this condition as well as the variable and simply use a break in the if statement.

About using append. You are appending numbers and popping them if they are not primes. Not needed. Just append at the correct condition and use break

Also read the Python performance tips link given in Python Tags' wiki on codereview. I would have written it like this:

import math
def primeList(listLength):
 if listLength < 1:
 return []
 plist = [2]
 j = 3
 sqr_root = math.sqrt
 list_app = plist.append
 while listLength > len(plist):
 temp = sqr_root(j)
 for i in xrange(len(plist)):
 if j % plist[i] == 0:
 break
 if plist[i] > temp:
 list_app(j)
 break
 
 j += 2
 return plist

Notice that I defined math.sqrt as something. You'll find that in the Python Performance tips. Also your implementation had a bug. If I entered anything less than 2 it returned [2, 3] which was incorrect result.

This worked in 44 % time that your original function took. Your code's timing and my code's timing . Note that memory usage is lower in my case.

Now done with micro-optimizations I'll get to major optimizations.

Using this approach to find the list of prime numbers is actually very naive. I also used it in the beginning and after much headache and waiting for outputs to come I found that such approaches can be very slow. Nothing you change in Python's syntax can offset the advantage of using a better algorithm. Check out this. It is not to difficult to implement. You can look at my github to get a basic implementation. You'll have to tweak it but you won't have to write it from the beginning.

Hope this helped.

I'll firstly talk about micro-optimizations then go for major optimizations that can be done in your code.

Don't use the while loop when you can instead use for i in xrange(). It is very slow. Take a look here and here. There are some limitations butxrange unless you are hitting that limit your code would get faster.

The inner while loop's conditions can be improved. As you have placed calculating math.sqrt() in the conditions it is calculated every time. That makes it very slow because finding square root consumes much time. You should use a variable before the loop to store that value. The second condition is not needed. Instead of checking always for the value of prime you can delete this condition as well as the variable and simply use a break in the if statement.

About using append. You are appending numbers and popping them if they are not primes. Not needed. Just append at the correct condition and use break

Also read the Python performance tips link given in Python Tags' wiki on codereview. I would have written it like this:

import math
def primeList(listLength):
 if listLength < 1:
 return []
 plist = [2]
 j = 3
 sqr_root = math.sqrt
 list_app = plist.append
 while listLength > len(plist):
 temp = sqr_root(j)
 for i in xrange(len(plist)):
 if j % plist[i] == 0:
 break
 if plist[i] > temp:
 list_app(j)
 break
 
 j += 2
 return plist

Notice that I defined math.sqrt as something. You'll find that in the Python Performance tips. Also your implementation had a bug. If I entered anything less than 2 it returned [2, 3] which was incorrect result.

Now done with micro-optimizations I'll get to major optimizations.

Using this approach to find the list of prime numbers is actually very naive. I also used it in the beginning and after much headache and waiting for outputs to come I found that such approaches can be very slow. Nothing you change in Python's syntax can offset the advantage of using a better algorithm. Check out this. It is not to difficult to implement. You can look at my github to get a basic implementation. You'll have to tweak it but you won't have to write it from the beginning.

Hope this helped.

I'll firstly talk about micro-optimizations then go for major optimizations that can be done in your code.

Don't use the while loop when you can instead use for i in xrange(). It is very slow. Take a look here and here. There are some limitations butxrange unless you are hitting that limit your code would get faster.

The inner while loop's conditions can be improved. As you have placed calculating math.sqrt() in the conditions it is calculated every time. That makes it very slow because finding square root consumes much time. You should use a variable before the loop to store that value. The second condition is not needed. Instead of checking always for the value of prime you can delete this condition as well as the variable and simply use a break in the if statement.

About using append. You are appending numbers and popping them if they are not primes. Not needed. Just append at the correct condition and use break

Also read the Python performance tips link given in Python Tags' wiki on codereview. I would have written it like this:

import math
def primeList(listLength):
 if listLength < 1:
 return []
 plist = [2]
 j = 3
 sqr_root = math.sqrt
 list_app = plist.append
 while listLength > len(plist):
 temp = sqr_root(j)
 for i in xrange(len(plist)):
 if j % plist[i] == 0:
 break
 if plist[i] > temp:
 list_app(j)
 break
 
 j += 2
 return plist

Notice that I defined math.sqrt as something. You'll find that in the Python Performance tips. Also your implementation had a bug. If I entered anything less than 2 it returned [2, 3] which was incorrect result.

This worked in 44 % time that your original function took. Your code's timing and my code's timing . Note that memory usage is lower in my case.

Now done with micro-optimizations I'll get to major optimizations.

Using this approach to find the list of prime numbers is actually very naive. I also used it in the beginning and after much headache and waiting for outputs to come I found that such approaches can be very slow. Nothing you change in Python's syntax can offset the advantage of using a better algorithm. Check out this. It is not to difficult to implement. You can look at my github to get a basic implementation. You'll have to tweak it but you won't have to write it from the beginning.

Hope this helped.

Source Link
Aseem Bansal
  • 2.3k
  • 3
  • 22
  • 37
Loading
lang-py

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