Little python script: Use separate functions?

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()

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.

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.

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.

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.

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.

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.

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.

Here is what I'd do :)

sum(filter(lambda a: a%3 == 0 or a %5 == 0, range(1000)))