132

Think about a function that I'm calling for its side effects, not return values (like printing to screen, updating GUI, printing to a file, etc.).

def fun_with_side_effects(x):
    ...side effects...
    return y

Now, is it Pythonic to use list comprehensions to call this func:

[fun_with_side_effects(x) for x in y if (...conditions...)]

Note that I don't save the list anywhere

Or should I call this func like this:

for x in y:
    if (...conditions...):
        fun_with_side_effects(x)

Which is better and why?

alani
  • 12,573
  • 2
  • 13
  • 23
sinan
  • 6,809
  • 6
  • 38
  • 67
  • 10
    This is an easy choice. Readability counts - do it the second way. If you can't fit 2 extra lines on your screen get a bigger monitor :) – John La Rooy Apr 22 '11 at 09:18
  • 1
    The list comprehension is unpythonic since it violates "explicit is better than implicit" -- you're hiding a loop in a different construct. – Fred Foo Apr 22 '11 at 09:42
  • 3
    @larsmans: if only GvR had realised that when he introduced list comprehensions in the first place! – Steve Jessop Apr 22 '11 at 13:21
  • 2
    @larsmans, Steve Jessop, I think it's incorrect to conceive a list comprehension as a loop. It may well be implemented as a loop, but the point of constructs like this is to operate on aggregate data in a functional and (conceptually) parallel way. If there's a problem with the syntax, it's that `for ... in` is used in both cases -- leading to questions like this one! – senderle Apr 23 '11 at 15:39
  • 2
    @senderle: I think it depends on the side-effects, though. If the side-effects just alter one element at a time, independently, then I think it's totally reasonable to use functional-style constructs for that in an imperative language, because it's not the loop flow-control that's important, it's the application to every element. If the side-effects are such that order matters, possibly then the "comprehension" abstraction is starting to leak. Whether it's leaking enough to matter is another question, though - nobody's pretending Python does lazy evaluation. – Steve Jessop Apr 24 '11 at 13:32
  • 1
    @Steve Jessop, perhaps -- my point was simply that I don't think list comprehensions, properly conceived, "hide a loop" as larsmans suggested. GvR's mistake (if he made one at all) was using "for" to indicate a loop in one case, and to indicate 'comprehension' in another case. But frankly I don't think he was making a mistake; I prefer python's list-comprehension syntax to Haskell's, for example, because I think it's more readable. – senderle Apr 24 '11 at 16:47

7 Answers7

112

It is very anti-Pythonic to do so, and any seasoned Pythonista will give you hell over it. The intermediate list is thrown away after it is created, and it could potentially be very, very large, and therefore expensive to create.

Tim Pietzcker
  • 328,213
  • 58
  • 503
  • 561
Ignacio Vazquez-Abrams
  • 776,304
  • 153
  • 1,341
  • 1,358
45

You shouldn't use a list comprehension, because as people have said that will build a large temporary list that you don't need. The following two methods are equivalent:

consume(side_effects(x) for x in xs)

for x in xs:
    side_effects(x)

with the definition of consume from the itertools man page:

def consume(iterator, n=None):
    "Advance the iterator n-steps ahead. If n is none, consume entirely."
    # Use functions that consume iterators at C speed.
    if n is None:
        # feed the entire iterator into a zero-length deque
        collections.deque(iterator, maxlen=0)
    else:
        # advance to the empty slice starting at position n
        next(islice(iterator, n, n), None)

Of course, the latter is clearer and easier to understand.

Katriel
  • 120,462
  • 19
  • 136
  • 170
  • @Paul: I think it should be. And indeed you can, though `map` may not be as intuitive if one hasn't done functional programming before. – Katriel Apr 22 '11 at 14:25
  • 4
    Not sure this is especially idiomatic. There's no advantage over using the explicit loop. – Marcin Jul 24 '13 at 18:12
  • 1
    Solution is `consume = collections.deque(maxlen=0).extend` – PaulMcG Oct 04 '18 at 16:43
29

List comprehensions are for creating lists. And unless you are actually creating a list, you should not use list comprehensions.

So I would got for the second option, just iterating over the list and then call the function when the conditions apply.

Ikke
  • 99,403
  • 23
  • 97
  • 120
  • 8
    I would go even further and state that side effects inside a list comprehension are unusual, unexpected, and therefore evil, even if you're using the resulting list when you're done. – Mark Ransom Feb 17 '17 at 19:38
13

Second is better.

Think of the person who would need to understand your code. You can get bad karma easily with the first :)

You could go middle between the two by using filter(). Consider the example:

y=[1,2,3,4,5,6]
def func(x):
    print "call with %r"%x

for x in filter(lambda x: x>3, y):
    func(x)
user237419
  • 8,829
  • 4
  • 31
  • 38
  • 1
    You don't even need filter. Just put a generator expression in parens here: `for el in (x for x in y if x > 3):`. `el` and `x` can have the same name, but that might confuse people. – Omnifarious Nov 01 '18 at 19:14
  • Note that those two `x`s are in different scopes, which is bad practice. – wjandrea Jun 15 '21 at 04:32
4

Depends on your goal.

If you are trying to do some operation on each object in a list, the second approach should be adopted.

If you are trying to generate a list from another list, you may use list comprehension.

Explicit is better than implicit. Simple is better than complex. (Python Zen)

rubayeet
  • 9,269
  • 8
  • 46
  • 55
  • The best answer, in principle. Syntax obeys to rules, so that one can answer "right", "wrong", "this way", "that way". Style obeys to other considerations, such as common sense, elegance, conciseness, effectiveness, efficiency, etc. Those depend on one's purposes, and even on one's taste. – fralau Oct 03 '20 at 06:35
0

You can do

for z in (fun_with_side_effects(x) for x in y if (...conditions...)): pass

but it's not very pretty.

joaquin
  • 82,968
  • 29
  • 138
  • 152
sigs
  • 17
  • 1
0

Using a list comprehension for its side effects is ugly, non-Pythonic, inefficient, and I wouldn't do it. I would use a for loop instead, because a for loop signals a procedural style in which side-effects are important.

But, if you absolutely insist on using a list comprehension for its side effects, you should avoid the inefficiency by using a generator expression instead. If you absolutely insist on this style, do one of these two:

any(fun_with_side_effects(x) and False for x in y if (...conditions...))

or:

all(fun_with_side_effects(x) or True for x in y if (...conditions...))

These are generator expressions, and they do not generate a random list that gets tossed out. I think the all form is perhaps slightly more clear, though I think both of them are confusing and shouldn't be used.

I think this is ugly and I wouldn't actually do it in code. But if you insist on implementing your loops in this fashion, that's how I would do it.

I tend to feel that list comprehensions and their ilk should signal an attempt to use something at least faintly resembling a functional style. Putting things with side effects that break that assumption will cause people to have to read your code more carefully, and I think that's a bad thing.

Omnifarious
  • 54,333
  • 19
  • 131
  • 194
  • What if `fun_with_side_effects` returns True? – Katriel Apr 22 '11 at 08:39
  • 8
    I think this cure is worse than the disease - itertools.consume is much cleaner. – PaulMcG Apr 22 '11 at 11:58
  • @PaulMcG - `itertools.consume` no longer exists, probably because using comprehensions with side-effects is ugly. – Omnifarious Nov 01 '18 at 19:05
  • 1
    It turns out I was mistaken, and it *never* existed as a method in the stdlib. It *is* a recipe in the itertools docs: https://docs.python.org/3/library/itertools.html?highlight=itertools#itertools-recipes – PaulMcG Nov 01 '18 at 19:18