1

There is a list of objects, "games." How can I check to see if the object has an attribute set, and if it doesn't, set the attribute.... using list comprehensions?

for g in games:
        if not g.score_ratio_h1: g.score_ratio_h1 = avg_score_ratio_h1
appleLover
  • 14,835
  • 9
  • 33
  • 50
  • 3
    In my opinion this is not a very good use case for a list comprehensions. List comprehensions are the best fit when you are doing functional-style filtering and projection that doesn't modify the original elements. Here, you are mutating each game. – recursive Jul 24 '13 at 16:08
  • 1
    If you don't see an `append()` in your loop somewhere, it's probably not feasible as a list comprehension. – kindall Jul 24 '13 at 16:10
  • 1
    Following link is about why list comprehension with side effect should be avoided http://stackoverflow.com/questions/5753597/is-it-pythonic-to-use-list-comprehensions-for-just-side-effects – Ansuman Bebarta Jul 24 '13 at 16:18

2 Answers2

4

This is not a good case for using list comprehensions, in fact: it's very anti-Pythonic. The loop doesn't result in the creation of a new list of values, it's just a sequence of assignments. Better stick to using a loop, it's fine as it is. Only if your code looked like this:

ans = []
for g in games:
    if not g.score_ratio_h1:
        ans.append(g.score_ratio_h1) # we're appending the results

... Then it'd be a good idea to use comprehensions. But currently the core of the loop is an assignment:

g.score_ratio_h1 = avg_score_ratio_h1

And no useful value returns of that, it's a modification operation (a "side effect") that doesn't get collected anywhere. Comprehensions are not meant to be used in such cases. Even more: trying to do an assignment inside a comprehension will result in an error, for example:

lst = [[0], [0], [0]]
[a[0] = 1 for a in lst]
      ^
SyntaxError: invalid syntax
Community
  • 1
  • 1
Óscar López
  • 232,561
  • 37
  • 312
  • 386
0

well you can do something like this that uses list comprehension:

for g in (g for g in games if not g.score_ratio_h1):
    g.score_ratio_h1 = avg_score_ratio_h1

it may be perhaps a bit faster... but strange :)

EDIT:

I agree with the two comments, however it may not be completely wasteful depending on the "if" condition, here an example:

  lst = [0 for _ in xrange(708)]
  for i in xrange(100000000):
      if i**2 < 500000:
          lst[i] += i

time:

real    0m12.906s
user    0m12.876s
sys     0m0.008s

versus:

lst = [0 for _ in xrange(708)]
for i in (i for i in xrange(100000000) if i**2 < 500000):
    lst[i] += i

time:

real    0m8.857s
user    0m8.792s
sys 0m0.016s

I guess that depending on the condition, and the size of the loop this may be indeed wasteful, but in sometimes it may help to play around list comprehension, even in this case.

fransua
  • 1,559
  • 13
  • 30
  • 3
    This will create a new list that gets discarded immediately after the iteration ends. It's wasteful, I don't think it's a good idea using comprehensions just for the heck of it, when a "traditional" loop will suffice. – Óscar López Jul 24 '13 at 16:16
  • 1
    Loop are always costly and in the above solution there are two for loops but the code in question has one loop. – Ansuman Bebarta Jul 24 '13 at 16:21
  • 1
    If you would put a generator expression here instead of a list comprehension, it would be fine: `for g in (g for g in games if not g.score_ratio_h1):` Just replace the `[]` by `()`. – glglgl Jul 25 '13 at 13:29
  • aps! I understand better the other comments now. thanks @glglgl :) – fransua Jul 25 '13 at 14:01