Is it valid to use conditional expressions for side effects?

Question

I'm trying to figure out what the best way of doing this is:

resource['contents'][media_type] = []
resource['contents'][media_type].append(row[0].toPython()) if row[0] is not None else None
resource['contents'][media_type].append(row[2].toPython()) if row[2] is not None else None

I think the code is quite simple; if the rows have a value then add them to the list. Is this approach considered OK? Is there any other approach that would be better? The toPython method will return the string description of the contained object.

Answer 1

Using a "ternary" conditional expression (x if C else y) for side effects is not at all Pythonic. Here's how I would do it:

resource['contents'][media_type] = []
for index in (0, 2):
    item = row[i]
    if item is not None:
        resource['contents'][media_type].append(item.toPython())

or using a list comprehension to reduce verbosity:

resource['contents'][media_type] = [row[i].toPython() for i in (0, 2) 
                                    if row[i] is not None]

These approaches are much more readable, and reduce duplication.

Answer 2

I don't think it is considered good practice to do that. What you could do instead is:

resource['contents'][media_type] = []

for irow in [0, 2]:
    if row[irow] is not None:
        resource['contents'][media_type].append(row[irow].toPython())

This allows you the flexibility of also using ranges (for irow in range(5)), or using rows if you can access them directly (for row in rows:).

Answer 3

No, that's not a valid use of a conditional expression. It confuses anyone trying to read your code.

Use an if statement; you can save some space by creating another reference to the list:

lst = resource['contents'][media_type] = []
if row[0] is not None: lst.append(row[0].toPython()) 
if row[2] is not None: lst.append(row[2].toPython())

but use a better name for the local reference (contents perhaps?), or use a list comprehension:

resource['contents'][media_type] = [
    col.toPython() for col in (row[0], row[2]) if col is not None]