List comprehensions for side effects - idiomatically correct or an abomination?


This question already has an answer here:


List comprehensions are for creating a list. That's not what you're doing here, so I'd avoid it.

Personally, I would do

for cheese in cheeses:

This feels a bit more explicit to me. It's another line, but I suspect that another programmer reading that code would understand it well, whereas the list comprehension version might be slightly confusing. In addition, I could imagine that someone else might come along later (not knowing that out_of_stock doesn't return a value) and try to use that list later.

I guess there's nothing wrong with it, though. Ultimately, it comes down to who's going to be reading that code later (yourself or others) and whether or not they'll know what's going on.

EDIT: Also, you're using a lovely functional programming construct taken from Haskell to do nasty stateful things...

Semantically, if cheese.out_of_stock returned a boolean, then it would be fine. However this is not very recommended if cheese.out_of_stock does not return a value, then it would be very confusing and unmaintainable.

import this

for more information

List comprehensions are usually used for filtering (or something similar where the result of the comprehsion is being used later) but not for replacing a loop calling a method on the iteration data. So this works but it locks ugly.

If you are doing something with the return values of cheese.out_of_stock() this is perfectly Pythonic. If you are discarding the result of the list comprehension, it is bad style.

Creating the list is a waste of time and memory, just use the two line for loop from Peter's answer

The answer hinges entirely on whether you want the list. A list comprehension is often a good replacement for a loop like this:

stock = []
for cheese in cheeses:

But rarely if ever for a loop like this:

for cheese in cheeses:

When people read a list comprehension, they don't typically expect it to have side effects - aside from something like cacheing that doesn't affect the meaning of the program, and, of course, building a list. Certainly, any side effects it has shouldn't be necessary for your program's correctness - this also means they can be worth avoiding for the first type of loop if you're using an API without command-query separation - that is, even if you do want the list, if .out_of_stock() also has side-effects that you depend on, you should consider using the loop rather than the comprehension.

It's ugly, don't do it.

There is a quite similar way of doing it that is not ugly. It requires a pre-defined function like this:

def consume(iter):
    for i in iter:

Now you can write code like this:

consume(cheese.out_of_stock() for cheese in cheeses)

So really, this is just Peters suggestion wrapped in a function which is called with a generator expression. And though much better than the list comprehension, it's still not exactly beautiful. It would really be best (most explicit) to just use the for loop directly.

for cheese in cheeses: cheese.out_of_stock()

Still a one liner and avoids the side effect of creating an unnecessary list.

Need Your Help

Overloading "+" in c++

c++ overloading

i have a problem with overloading the addition.

Getting the AudioCall in a Lync Conference via UCMA

call lync ucma conference

I'm looking for a way to get a references to AudioCalls in existing conferences via a Lync UCMA application so that I can change the audio routing for participants of a conference without them havi...