-1

Im trying to create a menu-driven program that will calculate and display all the prime numbers between 2 and a user-entered limit. The first option the user could pick would be "Create a list of primes from 2 to n using the Sieve of Eratosthenes algorithm". The second option would be "Display the prime numbers", and the third would be to quit. My code currently looks like:

def main():
    choice = displayMenu()
    while choice!= '3':
        if choice == '1':
            sieve()
        elif choice == '2':
            prime
        choice = displayMenu()

    print("Thanks for playing!")
def displayMenu():
    myChoice = '0'
    while myChoice != '1' and myChoice != '2' and myChoice != '3':
        print("""Please choose
                      1. Create a list of primes from 2 to n using
                      the Sieve of Eratosthenes algorithm
                      2. Display the prime numbers
                      3. Quit
                      """)
        myChoice = input("Enter option--->")

        if myChoice != '1' and myChoice != '2' and myChoice != '3':
            print("Invalid option. Please select again.")

    return myChoice

def sieve():
    liste = [ ]
    n = int(input("What number should I go up to?"))
    choice = input("--->")
    for primeCandidate in range (2,n):
        isPrime = True
        for divisor in range (2, primeCandidate):
            if primeCandidate % divisor == 0:
                isPrime = False
                break
            if isPrime:
                liste.append(primeCandidate)
                print(liste)
main()

What I am finding is that when I print the primes from 2 to 13 for example is that it prints it as

[3]
[3, 5]
[3, 5, 5]
[3, 5, 5, 5]
[3, 5, 5, 5, 7]
[3, 5, 5, 5, 7, 7]
[3, 5, 5, 5, 7, 7, 7]
[3, 5, 5, 5, 7, 7, 7, 7]
[3, 5, 5, 5, 7, 7, 7, 7, 7]
[3, 5, 5, 5, 7, 7, 7, 7, 7, 9]
[3, 5, 5, 5, 7, 7, 7, 7, 7, 9, 11]
[3, 5, 5, 5, 7, 7, 7, 7, 7, 9, 11, 11]
[3, 5, 5, 5, 7, 7, 7, 7, 7, 9, 11, 11, 11]
[3, 5, 5, 5, 7, 7, 7, 7, 7, 9, 11, 11, 11, 11]
[3, 5, 5, 5, 7, 7, 7, 7, 7, 9, 11, 11, 11, 11, 11]
[3, 5, 5, 5, 7, 7, 7, 7, 7, 9, 11, 11, 11, 11, 11, 11]
[3, 5, 5, 5, 7, 7, 7, 7, 7, 9, 11, 11, 11, 11, 11, 11, 11]
[3, 5, 5, 5, 7, 7, 7, 7, 7, 9, 11, 11, 11, 11, 11, 11, 11, 11]
[3, 5, 5, 5, 7, 7, 7, 7, 7, 9, 11, 11, 11, 11, 11, 11, 11, 11, 11]

Is there a way to correct that and only print each prime number once?

A. Gunter
  • 102
  • 1
  • 1
  • 13
  • 1
    Related: http://stackoverflow.com/questions/2068372/fastest-way-to-list-all-primes-below-n – tobias_k Mar 16 '17 at 17:21
  • I did see that one but for some reason I couldn't wrap my head around it! – A. Gunter Mar 16 '17 at 17:23
  • 1
    Split your code into code that handles input and output, and code which 'does the work'. Your `sieve` function both asks for input and performs the sieving. If you pass `n` as a parameter you can just test the function without needing to run the rest of the code. – Peter Wood Mar 16 '17 at 17:23

2 Answers2

3

The indentation in your sieve function is wrong. With this, you add the prime candidates to the list in each iteration of the loop and immediately print the list. Instead, you should add the prime candidates only after you tested all the divisors, i.e. after the inner loop, and print the numbers only when you found all the primes, i.e. after the outer loop.

Try this (changed lines are marked):

def sieve():
    liste = [ ]
    n = int(input("What number should I go up to?"))
    choice = input("--->")
    for primeCandidate in range (2,n):
        isPrime = True
        for divisor in range (2, primeCandidate):
            if primeCandidate % divisor == 0:
                isPrime = False
                break
        if isPrime: # only check isPrime after the loop completed
            liste.append(primeCandidate)
    print(liste)  # only print the list once

Also, as noted in comments, you might want to split the actual algorithm from the user interaction parts, and you can use a more compact all function with a generator expression instead of that inner loop. And you don't have to check all the numbers up to primeCandidate; instead, just check the so-far known primes.:

def sieve(n): # pass n as parameter
    liste = [ ]
    for primeCandidate in range (2,n):
        # use all with generator, and only check known primes
        isPrime = all(primeCandidate % divisor != 0 for divisor in liste)
        if isPrime:
            liste.append(primeCandidate)
    return liste  # return, don't print

Finally, note that strictly speaking, this is not a Sieve of Erastothenes! A real sieve would "cross off" multiples of numbers identified as primes, while your code pretty much does the opposite, using trial-division. Both works, but division is generally slower than multiplication.

For an extensive discussion of different Prime number algorithms, see this related question.

Community
  • 1
  • 1
tobias_k
  • 74,298
  • 11
  • 102
  • 155
  • That did solve one of my problems of it repeating the numbers over and over again! However it still prints it as [3, 5, 5, 5, 7, 7, 7, 7, 7, 9, 11, 11, 11, 11, 11, 11, 11, 11, 11] – A. Gunter Mar 16 '17 at 17:26
  • Oops just kidding! I figured out what I did wrong! Thank you so much!!! – A. Gunter Mar 16 '17 at 17:27
  • 1
    I'm going to leave my answer, but this is obviously the right answer for this case. – James R Mar 16 '17 at 17:29
0

Have you looked into set? and then doing set.add for each new integer you want to add to the set?

For example:

In [24]: a=[1]

In [25]: set(a)
Out[25]: {1}

In [26]: b=[1,1]

In [27]: set(b)
Out[27]: {1}

In [28]: c=set(b)

In [29]: c.add(1)

In [30]: c
Out[30]: {1}

In [31]: c.add(2)

In [32]: c
Out[32]: {1, 2}
James R
  • 4,365
  • 3
  • 27
  • 40