7
\$\begingroup\$

Which version of this code is better?

Ver 1.

a = 0
for i in xrange( 1000 ):
    if i % 3 == 0 or i % 5 == 0:
        a += i
print a

Ver 2.

from operator import add
print reduce( add,
              ( i for i in xrange( 1000 )
                if i % 3 == 0 or i % 5 == 0 ) )

Things I like about version one is that it is more readable, but it creates requires a variable. If you had to choose, which one would you argue is better?

\$\endgroup\$
  • 1
    \$\begingroup\$ Version 3: Use the sum() function with the generator used in version 2. \$\endgroup\$ – Jeff Mercado Dec 16 '11 at 22:26
  • 1
    \$\begingroup\$ is list descriptor a correct name for that ? Should not be for-loops vs generator comprehensions ? \$\endgroup\$ – joaquin Dec 18 '11 at 1:11
  • \$\begingroup\$ @joaquin Yes, it should (and is now). Descriptors are something entirely different. \$\endgroup\$ – sepp2k Dec 18 '11 at 1:30
8
\$\begingroup\$

The second version can (and should) be written using sum instead of reduce(add,...), which is somewhat more readable:

sum(i for i in xrange( 1000 ) if i % 3 == 0 or i % 5 == 0)

That is the version that I'd prefer as far as implementations of this particular algorithm go because it's more declarative than a loop and just reads nicer in my opinion. (It's also faster though that's less of a concern).

However it should be noted that for summing all numbers up to n that divisible by 3 or 5, there's actually a better algorithm than going over all those numbers and summing them.

\$\endgroup\$
  • \$\begingroup\$ On testing, the generator version is not faster than a simple for loop. Do you have a reference / benchmarking to support your claim? \$\endgroup\$ – jpp Mar 1 '18 at 1:01
0
\$\begingroup\$

Don't use reduce, it is removed python 3.

kracekumar@python-lover:~$ python -m timeit 'sum(i for i in xrange(1000) if i%3 ==0 or i%5==0) '
1000 loops, best of 3: 232 usec per loop

kracekumar@python-lover:~$ python -m timeit 'sum([i for i in xrange(1000) if i%3 ==0 or i%5==0]) '

10000 loops, best of 3: 177 usec per loop

First method uses generators second one uses list comprehensions.

You can use both but generators will occupy less memory.

In [28]: g = (i for i in xrange(1000) if i%3 ==0 or i%5==0)

In [29]: l = [i for i in xrange(1000) if i%3 ==0 or i%5==0]

In [30]: import sys

In [31]: sys.getsizeof(l)
Out[31]: 2136

In [32]: sys.getsizeof(g)
Out[32]: 36

It is advised to use generators, here is a beautiful tutorial about generators.

\$\endgroup\$
  • 3
    \$\begingroup\$ reduce isn't removed in Python 3, just moved (into the functools module). \$\endgroup\$ – sepp2k Dec 17 '11 at 20:23

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.