CODEWITHSUNDEEP
pythonpython-3.x
vrund_
vrund_

Reputation: 3

Having trouble sending variables in functions as arguments

I am making simple program to get all the prime numbers in a range provided by the user. I get the input and I am attempting to send the lower bound and upper bound to a function. However, it keeps stating that the first parameter value is not used. The function is called prime_checker.

Strangely, I believe that the method I am using to send the variables over is correct as the second parameter is used. I tried using the same variables names (lower_bound) yet it does not function.

def prime_checker(lower, upper):
    x = 2
    flag = True
    for lower in range(upper):
        for x in range(lower):
            if lower % x == 0:
                flag = False
                x += 1
                lower += 1
            else:
                print(lower + "is a prime number.")
                x += 1
                lower += 1

lower_bound = int(input("Enter a lower bound: "))
upper_bound = int(input("Enter an upper bound: "))
prime_checker(lower_bound, upper_bound)

line 19, in <module>
    prime_checker(lower_bound, upper_bound)

line 8, in prime_checker
    if lower % x == 0:
ZeroDivisionError: integer division or modulo by zero

Paramerer 'lower' is not used

Upvotes: 0

Views: 74

Answers (2)

norok2
norok2

Reputation: 26886

The issue is that you are doing lower % x when both lower and x are 0. The value of lower and x is determined by the earlier lines for .. in range(...), which in Python let you loop from 0 to the number you specify as parameter in range() minus one. Hence, exactly in the first iteration you have 0 % 0, which is an undefined operation.

EDIT: That is not the only issue present in your code, e.g.:

  • you are incrementing both lower and x explicitly, but this is actually taken care of by the for construction;
  • you do some operations identically on both branches of the if and that's unnecessarily redundant;
  • the algorithm is not really effective at checking for primality right now;
  • your function always return None since it is missing an explicit use of return
  • etc.

Along with what I believe is your train of thought, I would have probably written something like:

def give_primes(lower, upper):
    result = []
    for n in range(lower, upper):
        is_prime = True
        # check if `n` is prime
        # this could be heavily optimized
        for m in range(2, n):
            if n % m == 0:
                is_prime = False
                break
        if is_prime:
            result.append(n)
    return result


lower_bound = int(input("Enter a lower bound: "))
# you input: 10
upper_bound = int(input("Enter an upper bound: "))
# you input: 20

print(give_primes(lower_bound, upper_bound))
# [11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47]

This could be further improved, e.g. by using a separate is_prime() function (whose efficiency could be heavily improved compared to what is reported above, e.g. by checking % 2 only once and then using step=2 on the inner range(), stopping not when m >= n but when m >= sqrt(n), etc.), using a generator instead of returning a list, etc. but that would be too much meat for the grill of this question.

Upvotes: 0

Mad Physicist
Mad Physicist

Reputation: 114230

The statement for lower in range(upper): assigns every value between zero (inclusive) and upper exclusive to the name lower. This discards the original value you pass in without ever using it. A corrected version would be

for lower in range(lower, upper + 1):

This takes care of iterating over the correct range.

Your inner loop has a couple of problems too. It starts at zero, which is the immediate cause of your error. You shouldn't start at one either, since divisibility by one is guaranteed. Instead, start with two:

for x in range(2, lower):

You definitely want to be exclusive on lower, since divisibility by that is guaranteed as well.

When lower % x == 0, you don't want to increment either variable, since the loops will overwrite any changes you make. Instead, you want to skip to the next iteration of lower, and start checking x from the beginning. This is accomplished by the break statement, which immediately exits the inner loop, and continues with the outer:

if lower % x == 0:
    break

Notice that just because a number is not divisible by 2, it is not necessarily prime. Your current else statement will declare lower prime for every value of x that isn't a divisor, regardless of whether lower is prime or not. The correct thing to do would be to report primeness only once, after checking all possible values of x.

Python happens to have a special version of the else clause that goes after a for loop, and only gets triggered when the loop completes without a break. So all you have to do is unindent your else one level, and of course don't pointlessly set loop variables.

The final result is something like this:

for lower in range(lower, upper + 1):
    for x in range(2, lower):
        if lower % x == 0:
            break
    else:
        print(f'{lower} is prime')

This is not a particularly efficient way to search for primes, but it illustrates how to fix the programming issues in your code.

Upvotes: 1

Related Questions