Using separate functions for Project Euler 1

Question

I started programming with Java and C++, so I'm used to having a 'main' function that calls other functions that do the actual work. At university I was always told that doing actual computation in the main function is bad practice. I'm currently playing around with Python, and I have trouble figuring out how to write a nice 'main' function, especially since I'm doing small stuff that doesn't need separate classes.

What do you think about the following code? Is the main function necessary, or would you just write everything without functions? Is there a general consent on this in the Python world?

# Finds sum of all multiples of 3 and 5 from 0 to 999 def find_multiples(): global numbers for i in range(0,1000): if i%3==0 or i%5==0: numbers.append(i); numbers = [] if __name__ == '__main__': find_multiples() print sum(numbers) 

Answer 1

In most of the Python code I've ever seen, there is normally a main function defined that you call from the condition you put to establish the script was executed and not imported. I believe that's the standard way of doing things in Python, but I don't believe its actually a written rule anywhere, though that is how they do it in their docs as well.

Answer 2

Here's some superficial review. More for testing the site and some comment formatting than anything, but: do create main functions (helps us benchmarkers a lot) and do think that your module can be imported, so docstrings and local variables help.

# Finds... ###-^ Put this in a docstring def find_multiples(): """Finds sum of all multiples of 3 and 5 from 0 to 999 """ ###-^ This allows you to "from x import find_multiples, help(find_multiples)" numbers = [] ###-^ Avoid globals for i in xrange(1000): ###-^ Use xrange if Python 2.x, no need to start at 0 (it's the default) if not (i % 3) or not (i % 5): ###-^ Add spaces around operators, simplify ###-^ the boolean/numeric checks numbers.append(i) ###-^ Remove trailing ; return numbers ###-^ Removed global def main(): ###-^ Allows calling main many times, e.g. for benchmarking numbers = find_multiples() print sum(numbers) if __name__ == '__main__': main() 

Answer 3

The UNIX Man's recommendation of using a generator rather than a list is good one. However I would recommend using a generator expressions over yield:

def find_multiples(min=0, max=1000): """Finds multiples of 3 or 5 between min and max.""" return (i for i in xrange(min, max) if i%3==0 or i%5==0) 

This has the same benefits as using yield and the added benefit of being more concise. In contrast to UNIX Man's solution it also uses "positive" control flow, i.e. it selects the elements to select, not the ones to skip, and the lack of the continue statement simplifies the control flow¹.

On a more general note, I'd recommend renaming the function find_multiples_of_3_and_5 because otherwise the name suggests that you might use it to find multiples of any number. Or even better: you could generalize your function, so that it can find the multiples of any numbers. For this the code could look like this:

def find_multiples(factors=[3,5], min=0, max=1000): """Finds all numbers between min and max which are multiples of any number in factors""" return (i for i in xrange(min, max) if any(i%x==0 for x in factors)) 

However now the generator expression is getting a bit crowded, so we should factor the logic for finding whether a given number is a multiple of any of the factors into its own function:

def find_multiples(factors=[3,5], min=0, max=1000): """Finds all numbers between min and max which are multiples of any number in factors""" def is_multiple(i): return any(i%x==0 for x in factors) return (i for i in xrange(min, max) if is_multiple(i)) 

---

¹ Of course the solution using yield could also be written positively and without continue.

Answer 4

Here's how I would do it:

def find_multiples(min=0, max=1000): """Finds multiples of 3 or 5 between min and max.""" for i in xrange(min, max): if i%3 and i%5: continue yield i if __name__ == '__main__': print sum(find_multiples()) 

This makes find_multiples a generator for the multiples it finds. The multiples no longer need to be stored explicitly, and especially not in a global.

It's also now takes parameters (with default values) so that the caller can specify the range of numbers to search.

And of course, the global "if" block now only has to sum on the numbers generated by the function instead of hoping the global variable exists and has remained untouched by anything else that might come up.

Answer 5

Usually everything in python is handled the come-on-we-are-all-upgrowns principle. This allows you to choose the way you want things to do. However it's best practice to put the main-function code into a function instead directly into the module.

This makes it possible to import the function from some other module and makes simple scripts instantly programmable.

However avoid the use of globals (what you did with numbers) for return values since this makes it difficult to execute the function a second time.

Answer 6

Regarding the use of a main() function.

One important reason for using a construct like this:

if __name__ == '__main__': main() 

Is to keep the module importable and in turn much more reusable. I can't really reuse modules that runs all sorts of code when I import them. By having a main() function, as above, I can import the module and reuse relevant parts of it. Perhaps even by running the main() function at my convenience.

Answer 7

Just in case you're not familiar with the generator technique being used above, here's the same function done in 3 ways, starting with something close to your original, then using a list comprehension, and then using a generator expression:

# Finds sum of all multiples of 3 and 5 from 0 to 999 in various ways def find_multiples(): numbers = [] for i in range(0,1000): if i%3 == 0 or i%5 == 0: numbers.append(i) return numbers def find_multiples_with_list_comprehension(): return [i for i in range(0,1000) if i%3 == 0 or i%5 == 0] def find_multiples_with_generator(): return (i for i in range(0,1000) if i%3 == 0 or i%5 == 0) if __name__ == '__main__': numbers1 = find_multiples() numbers2 = find_multiples_with_list_comprehension() numbers3 = list(find_multiples_with_generator()) print numbers1 == numbers2 == numbers3 print sum(numbers1) 

find_multiples() is pretty close to what you were doing, but slightly more Pythonic. It avoids the global (icky!) and returns a list.

Generator expressions (contained in parentheses, like a tuple) are more efficient than list comprehensions (contained in square brackets, like a list), but don't actually return a list of values -- they return an object that can be iterated through.

So that's why I called list() on find_multiples_with_generator(), which is actually sort of pointless, since you could also simply do sum(find_multiples_with_generator(), which is your ultimate goal here. I'm just trying to show you that generator expressions and list comprehensions look similar but behave differently. (Something that tripped me up early on.)

The other answers here really solve the problem, I just thought it might be worth seeing these three approaches compared.

Answer 8

I would keep it simple. In this particular case I would do:

def my_sum(start, end, *divisors): return sum(i for i in xrange(start, end + 1) if any(i % d == 0 for d in divisors)) if __name__ == '__main__': print(my_sum(0, 999, 3, 5)) 

Because it is readable enough. Should you need to implement more, then add more functions.

There is also an O(1) version(if the number of divisors is assumed constant), of course.

Note: In Python 3 there is no xrnage as range is lazy.

Answer 9

Adding more to @pat answer, a function like the one below has no meaning because you can NOT re-use for similar tasks. (Copied stripping comments and docstring.)

def find_multiples(): numbers = [] for i in xrange(1000): if not (i % 3) or not (i % 5): numbers.append(i) return numbers 

Instead put parametres at the start of your function in order to be able to reuse it.

def find_multiples(a,b,MAX): numbers = [] for i in xrange(MAX): if not (i % a) or not (i % b): numbers.append(i) return numbers 

Answer 10

Here is one solution using ifilter. Basically it will do the same as using a generator but since you try to filter out numbers that don't satisfy a function which returns true if the number is divisible by all the factors, maybe it captures better your logic. It may be a bit difficult to understand for someone not accustomed to functional logic.

from itertools import ifilter def is_multiple_builder(*factors): """returns function that check if the number passed in argument is divisible by all factors""" def is_multiple(x): return all(x % factor == 0 for factor in factors) return is_multiple def find_multiples(factors, iterable): return ifilter(is_multiple_builder(*factors), iterable) 

The all(iterable) function returns true, if all elements in the iterable passed as an argument are true.

(x % factor == 0 for factor in factors) 

will return a generator with true/false for all factor in factors depending if the number is divisible by this factor. I could omit the parentheses around this expression because it is the only argument of all.