HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

User-input prime number generator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numberusergeneratorinputprime

Problem


  • What do you consider the readability of this program?



  • Do you think it's easy to understand what the variables and functions do just by reading their names?



  • Would you prefer more, less, or this amount of comments?



  • Do the comments make it easier or harder for you to understand the program?



  • Do you like a program to have comments (in general)?



```
# This program generates prime numbers

# Introduces user
def welcome():
print("""
Welcome to the prime number generator!

This program genarates all prime numbers starting from zero
to an upper limit. You´ll decide the upper limit.

Have fun :)
""")

# User inputs a natural number that will be the upper limit for checking primenumbers
def limit(question):
while True:
try:
limit = int(input(question))
except:
limit = ""
if limit:
return limit
break

# Checks if the number is a prime number. If it is, the number will be returned. If it is not, then None will be returned
def is_it_prime(current_number, prime_number):
y = 0
while True:
ratio = current_number / prime_number[y]
y += 1
if ratio == int(ratio):
break
if y == len(prime_number):
return current_number

# Adds prime number to list
def ad_prime(prime_number, it_is_prime):
a = len(prime_number)
prime_temp = (a+1)*[None]
for i in range(a):
prime_temp[i] = prime_number[i]
if i+1 == a:
prime_temp[i+1] = it_is_prime
return prime_temp

# Main program
upper_limit_question = "Type the upper limit and press enter. It has to be a natural number expressed in digits: "
lower_limit = 2
prime_number = [2]
welcome()
upper_limit = limit(upper_limit_question)
while lower_limit < upper_limit:
lower_limit +=1
it_is_prime = is_it_prime(lower_limit, prime_number)
if it_is_prime != None:
prime_number = ad_prime(prime_number, it_is_

Solution

It is easy to get a general understanding of what your code does. You did a good job separating the code into functions. It could be made more readable in a few ways though, mostly by doing things in a simpler way. For example, your add_prime function (which appends a number to the end of a list) could be written in one line (I'll get to how later).

I recommend reading the Python tutorial. By the time you get to chapter 4, you'll notice a lot of thing you would have done differently. :)

The variable names are good and make it easy to follow what you're doing, but the are a couple of names that are a bit confusing. In particular, prime_number should be prime_numbers or simply primes because it is a list of numbers. Also, limit isn't very descriptive at all. The difference between the limit function and the upper_limit variable is that one gets the value from the user, and the other stores it. limit should be get_limit or get_limit_from_user.

I like the way you comment. The amount of comments is good and they improve readability. Keep commenting like that. I have one note, though: When you're writing a comment that describes what a whole function or a whole program does, use a docstring. This makes it clearer to other programmers what you commenting about, and makes it possible to access the comment in the code.

Here is what an improved version of the code could look like (comments starting with ## are part of this review and don't belong to the program):

"""Generate prime numbers."""    ## This is a docstring.

WELCOME_TEXT = """Welcome to the prime number generator!

This program generates all prime numbers starting from zero to an upper limit.
You'll decide the upper limit.

Have fun :)
"""

LIMIT_PROMPT = ("Type the upper limit and press enter. It has to be a natural "
                "number expressed in digits: ")


ALL_CAPS_NAMES are a convention meaning a value should be treated as constant. In general, you should set constant values at the start of your program. In this case, this also has the advantage that the WELCOME_TEXT won't be indented.

The two strings in LIMIT_PROMPT will automatically be concatenated. This way, you can write a one-line string in two lines of code.

def welcome():
    """Introduce user."""    ## Also a docstring.  If your function definition starts 
    print(WELCOME_TEXT)      ## with a string, the string will be made a docstring.

def get_limit():
    """Let user input a an upper limit for checking prime numbers.

    The input must be a natural number."""

    while True:
        try:
            return int(input(LIMIT_PROMPT))
        except ValueError:
            pass


I made a few changes to the get_limit function. You should always catch only specific exceptions (ValueError in this case.) Also, I think it's better not to mix strings and numbers in one variable.

def is_prime(current_number, prime_numbers):
    """Check whether the number is a prime number.

    prime_numbers must contain all prime numbers lower than current_number."""

    for prime in prime_numbers:
        if current_number % prime == 0:    ## (current_number % prime) is the remainder
            return False                   ## when current_number is divided by prime.
    return True


You can loop over a list can be done easier this way. Making is_prime return a boolean will make things easier for you in the main part of the program.

def main():
    welcome()
    upper_limit = get_limit()
    prime_numbers = []
    for number in range(2, upper_limit+1):
        if is_prime(number, prime_numbers):
            prime_numbers.append(number)    ## This adds number to the end of prime_numbers,
                                            ## so you don't need add_prime anymore.
    print("\n", prime_numbers, "\n\nThanks for generating prime numbers :)")

main()


I put the main part of the program into a main function (this is a useful convention). Again, you can simplify the loop, this time using the range function. For simple loops, there is usually a way of using a very readable for loop instead of a while loop. I also changed the name lower_limit to number, which fits better in my opinion.

If you have any questions, feel free to ask them. Welcome to codereview. :)

Code Snippets

"""Generate prime numbers."""    ## This is a docstring.


WELCOME_TEXT = """Welcome to the prime number generator!

This program generates all prime numbers starting from zero to an upper limit.
You'll decide the upper limit.

Have fun :)
"""

LIMIT_PROMPT = ("Type the upper limit and press enter. It has to be a natural "
                "number expressed in digits: ")
def welcome():
    """Introduce user."""    ## Also a docstring.  If your function definition starts 
    print(WELCOME_TEXT)      ## with a string, the string will be made a docstring.


def get_limit():
    """Let user input a an upper limit for checking prime numbers.

    The input must be a natural number."""

    while True:
        try:
            return int(input(LIMIT_PROMPT))
        except ValueError:
            pass
def is_prime(current_number, prime_numbers):
    """Check whether the number is a prime number.

    prime_numbers must contain all prime numbers lower than current_number."""

    for prime in prime_numbers:
        if current_number % prime == 0:    ## (current_number % prime) is the remainder
            return False                   ## when current_number is divided by prime.
    return True
def main():
    welcome()
    upper_limit = get_limit()
    prime_numbers = []
    for number in range(2, upper_limit+1):
        if is_prime(number, prime_numbers):
            prime_numbers.append(number)    ## This adds number to the end of prime_numbers,
                                            ## so you don't need add_prime anymore.
    print("\n", prime_numbers, "\n\nThanks for generating prime numbers :)")

main()

Context

StackExchange Code Review Q#29412, answer score: 2

Revisions (0)

No revisions yet.