Making python code more efficient

Question

I have the code

num = 1
num2 = 1
num3 = 1
list = []
list2 = []
list3 = []

def numCheck1 (num):
    while num<1001:
        if (num%3==0):
            if (num%5==0):
                print num
                list.append(num)
                num+=1
                numCheck1(num)
                break
            else:
                print "error 1"
        else:
            print "error 2"
        num+=1

numCheck1(num)
total=sum(list)
print list
print total

def numCheck2 (num2):
    while num2<1001:
        if (num2%5==0):
            print num2
            list2.append(num2)
            num2+=1
            numCheck1(num2)
            break
        else:
            print "error"

numCheck2(num2)

def numCheck3 (num3):
    while num3<1001:
        if (num3%3==0):
            print num3
            list3.append(num3)
            num3+=1
            numCheck1(num3)
            break
        else:
            print "error"

numCheck3(num3)

total2 = sum(list2)
total3 = sum(list3)
overall = (total2 + total3) - total
print list2
print list3
print total2
print total3
print overall

As a basic summary of my code, I have 3 functions, and corresponding lists and variables for each of them. The first function checks for all multiples of 3 and 5 below and equal to 1000. The second checks for all multiples of 5 below and equal to 1000. The third checks for all multiples of 3 below and equal to 1000. The numbers that are multiples are added to the corresponding list, while the corresponding variable is incremented to allow the function to check all numbers. At the end, the program calculates 4 totals: the total of each of the lists, and a special total, which adds together the second two totals and subtracts the first to prevent overcounting. This is just the overarching structure of the program.

This program is supposed to solve this problem (not homework, just fun). The code is working (as far as I know; the first function definitely works) but it keeps crashing the compiler (I am using an online compiler, repl. I am wondering if there are any ways to make this code more efficient.

Thanks!

I think you reach maximum stack recursion depth , why you use a recursive function? while is sufficient for this purpose — ᴀʀᴍᴀɴ
– ᴀʀᴍᴀɴ, Commented Aug 17, 2016 at 0:56
A few things after a glance: You appear to be calling numCheck1 from inside itself. Also, don't assign a variable to the name list (or str, or anything else reserved). And finally, there are a lot of elements here that violate Python style, such as spacing and use of brackets - check out PEP8 for more. — Jeff
– Jeff, Commented Aug 17, 2016 at 0:57

Wayne Werner · Accepted Answer · 2016-08-17 01:13:36Z

I've re-written your code, with comments in-line. You could improve things even more over this, but I think seeing where you could improve this code will be helpful for you.

# If you use this, it should be the first line
# in your code. If you're using Python2, you *should*
# use this, because it will make switching to Python3
# that much easier.
from __future__ import print_function

# No need for these global variables
#num = 1
#num2 = 1
#num3 = 1
#list = []
#list2 = []
#list3 = []
#

# Functions should be snake_cased, not camelCase
# also, the blank space between the function name
# and the paren is inconsistent with typical
# Python code.
def num_check_one(start):
    # We'll put the return list in here. Also
    # calling it `list = []` would have shadowed
    # a builtin function. Let's give it a better name:
    multiples = []

    # If you're iterating over known values in Python
    # use a for loop over a range instead.
    for num in range(start, 1001):

        # No need to nest your ifs.
        # Parenthesis are usually just extra noise,
        # But you might find it a little clearer with
        # them to clarify the grouping
        #if (num % 3 == 0) and (num % 5 == 0):
        # This could also be written as
        # if (not num % 3) and (not num % 5):
        # Or, using De Morgan's law:
        # if not (num % 3 or num % 5):
        if num % 3 == 0 and num % 5 == 0:
            print(num)
            multiples.append(num)
            # Uh, no need to recursively call
            # your function here.
            #numCheck1(num)
            #break
        # And finally, you don't need any of these
#            else:
#                print "error 1"
#        else:
#            print "error 2"
#        num+=1
    return multiples


# You can use the keyword in your function call
# which makes it clear what the value is for
multiples = num_check_one(start=1)
total=sum(multiples)
print('Multiples:', multiples)
print('Total:', total)

# Again, fixed casing/spacing, and gave the
# parameter a meaningful name
def num_check_two(start):
    multiples = []
    # Again, for loop
    for num in range(start, 10001):
        # 0 is Falsey, so we can
        # just treat it as a bool value
        if not num % 5:
            print(num)
            multiples.append(num)
            # Got rid of the recursive call again
        # You were never incrementing `num2` here,
        # which is why your code got into an infinite
        # loop. Also why you should use `for` loops by
        # default

num_check_two(1)

Neil A. · Accepted Answer · 2016-08-17 00:57:07Z

1

numCheck1: Since all multiples of 3 and 5 are also multiples of 15, you can simply print out each item in range(0, 1001, 15) (counts by 15)

numCheck2: range(0, 1001, 5) (counts by 5) should already be all multiples of 5 less than or equal to 1000.

numCheck3: range(0, 1001, 3) (counts by 3) is the same thing as above, simply with multiples of 3.

answered Aug 17, 2016 at 0:57

Neil A.

8417 silver badges23 bronze badges

Collectives™ on Stack Overflow

Making python code more efficient

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related