0

The code prints every non-prime number twice but the prime-numbers only once. However, I only want the prime-numbers to be printed out. I know there are way better Solutions for a prime-number-generator but I really want to know where the mistake in this code is.

prime_numbers = []

def prime_gen(upper_limit):
    for i in range(2, upper_limit):
        for j in range(2, upper_limit):
            if i % j == 0 and i % j != 1 and i % j != j and i:
                prime_numbers.append(i)
prime_gen(20)
print(prime_numbers)
Ami Tavory
  • 66,807
  • 9
  • 114
  • 153
antoni
  • 19
  • 5

5 Answers5

1

You should stop j at i, not the upper limit. No point looking for divisors of i that are larger than i - there aren't any. And i itself shouldn't be tested, as it always divides itself.

And a number isn't prime because it's divisible by another but because it isn't. So test all possible divisors and only at the end, if none was found, only then add i to the prime number list.

prime_numbers = []

def prime_gen(upper_limit):
    for i in range(2, upper_limit):
        for j in range(2, i):             # <== only look for divisors less than i
            if i % j == 0:                # <== STOP if you found a divisor
                break
        else:                             # <== Add only if no divisor was found
            prime_numbers.append(i)
prime_gen(20)
print(prime_numbers)
Stefan Pochmann
  • 24,379
  • 7
  • 36
  • 92
1
 prime_numbers = [2] # we know two is prime
 def prime_gen(upper_limit):
    # start at 3 and use a step of 2
    for i in range(3, upper_limit, 2):
        # loop from 2 to i
        for j in range(2, i):
            # if i was divisible by any j we will break the loop
            # as i is not prime
            if i % j == 0:
                break
        else:
            # if we get here we completed our inner loop
            # which means no i % j was equal to 0
            prime_numbers.append(i)

You need the inner loop to go from 2 to i, you don't want numbers that satisfy if i % j == 0 as those are not prime. Your last and i is also always going to be True, any number that is not 0 will be True so the test is redundant. You can also start at 3 and use a step of 2, all even numbers cannot be prime.

You can also replace the if/else with any: which will lazily evaluate and break if we find any i % j that equals 0.

prime_numbers = [2]

def prime_gen(upper_limit):
    for i in range(3, upper_limit, 2):
        if not any(i % j == 0 for j in range(2, i)):
            prime_numbers.append(i)
Padraic Cunningham
  • 160,756
  • 20
  • 201
  • 286
  • @DavidBeck, no worries, you're welcome. If you do actually want an efficient way to generate primes, use a sieve of Eratosthenes http://stackoverflow.com/a/23423821/2141635 – Padraic Cunningham May 09 '15 at 19:04
0

First one is not optimised, while the second one is slightly more optimised. Of course, "Sieve of Eratosthenes" is the best. This functions produce the prime numbers in sequence, but not having an upper limit.

Simple and not optimised:

def simple_prime_list(num):
    list_of_prime = (2, )
    current_num = 2
    is_prime = True
    while len(list_of_prime) != num:
        current_num += 1
        for i in list_of_prime:
            if current_num % i == 0:
                is_prime = False
        if is_prime == True:
            list_of_prime += (current_num,)
        #To reset the status
        is_prime = True
    return list_of_prime

Slightly optimised by not checking all even number and break out of the for loop when the number is not a prime:

def prime_list(num):
    list_of_prime = (2, )
    current_num = 2
    is_prime = True
    while len(list_of_prime) != num:
        current_num += 1
        if current_num % 2 != 0:
            for i in list_of_prime:
                if current_num % i == 0:
                    is_prime = False
                    break
            if is_prime == True:
                list_of_prime += (current_num, )
        #To reset the status
        is_prime = True
    return list_of_prime

Try measuring the 2 different runtime:

import time
def measureTime(fn):
    start = time.clock()
    fn()
    end = time.clock()
    #return value in millisecond
    return (end - start)*1000

print('Simple Prime List:', measureTime(lambda: simple_prime_list(1000)), 'ms')
print('Optimised Prime List:', measureTime(lambda: prime_list(1000)), 'ms')

Output:

Simple Prime List: 775.798 ms

Optimised Prime List: 69.48299999999996 ms

Community
  • 1
  • 1
0

if you are looking for generator you should use yield instead of return, here is my answer for a infinite primes generator:

def primes_generator():
    n = 1
    while True:
        for x in range(2, n): 
            if n % x == 0:
                #not prime
                break
        else: 
            #prime
            yield n #<- yield instead of return to create a generator
        n+=1

if you need an upper limit you can use it like:

def primes_generator(upper_limit):
    for n in range(2, upper_limit):
        for x in range(2, n): 
            if n % x == 0: break
        else: 
            yield n

then you can use it like:

primes = primes_generator()
for prime in primes:
    print(i)
efirvida
  • 3,807
  • 2
  • 33
  • 58
0

Simple python console prime number generator

global num,primes
num = 1
primes=[]
gen = raw_input("enter the number of primes to generate: ")


while len(primes)<int(gen)+1:
    if num > 0:
        for i in range(2, num):
            if (num % i) == 0:
                num+=1
                break
    else:
        print(primes)
            primes.append(num)
            num+=1
print "Primes generated : ",len(primes)-1