How to convert a for loop with multiple statements in if, elif conditions to List Comprehensions

Question

for i in range(len(q)):
        # q.remove(max(q))
        # maxi = max(q)

    if((q.index(max(q)))+3) in range(len(q)):
            return("Too chaotic")

    if((q.index(max(q)))+2) in range(len(q)):
            bribe = bribe + 2
            q.remove(max(q))
    elif ((q.index(max(q)))+1) in range(len(q)):
            bribe = bribe + 1
            q.remove(max(q))
    elif(q.index(max(q))) == (len(q)-1):
            q.remove(max(q))
 return(bribe)

I want to convert the above for loop to list comprehension. I tried doing

["Too chaotic" if((q.index(max(q)))+3) in range(len(q)) 
 bribe+2,q.remove(max(q)) if((q.index(max(q)))+2) in range(len(q))  
 else [bribe+2,q.remove(max(q)]  if((q.index(max(q)))+2) in range(len(q)) 
 else q.remove(max(q)) if (q.inde  (max(q))) == (len(q)-1) for i in 
 range(len(q))]

but it did not work.

Don't. You are making it really, really hard on your future self. There is no need to convert everything to a list comprehension, not when it becomes an unreadable mess. — Martijn Pieters, Apr 06 '19 at 17:42
Note: `if` and `elif` are not functions, they don't need `(....)`. You are really overusing parentheses here, making it really hard to read what is going on in your code even without a list comprehension. — Martijn Pieters, Apr 06 '19 at 17:43
Last but not least, you are not even building a list here. List comprehensions are there to **create a list of values**. Don't use them just because they contain a loop. They are not a generic on-liner for-loop replacement! — Martijn Pieters, Apr 06 '19 at 17:44
`someinteger in range(len(q))` is also a really hard to parse way of saying `0 <= someinteger < len(q)`. Don't use ranges when you only need to check boundaries. — Martijn Pieters, Apr 06 '19 at 17:46
Ah, you are modifying the list, removing elements. It's a very confusing piece of code, it would at least have helped to understand what it was meant to do. — Martijn Pieters, Apr 06 '19 at 17:53
See https://gist.github.com/mjpieters/c8ea31acf36109821d89b7394dfaba66 for a clean-up of your posted code; it can probably be re-written using a sort with `enumerate()`, however. This is not something that's suitable for a list comp, either way. — Martijn Pieters, Apr 06 '19 at 17:59

score 0 · Answer 1 · answered Apr 14 '19 at 13:45

There are dozens of questions like this on SO: how can I turn this piece of code into a list/dict comprehension? I have seen three main goals:

performance: one wants to increase the speed or reduce the memory footprint of the code;
expressiveness: one wants a concise and clear code;
learning/fun/experimentation: one wants to learn about list comprehensionsself.

But sometimes, we simply don't know. This is the case here, hence I'll start with generic thoughts.

Here are my rules of thumb before creating a list comprehension, by order of importance:

Start with a clean code, because the list comprehension won't make your code cleaner (actually it's likely quite the opposite)
Be sure you want to create a list or an aggregate function of a list (sum, maximum, ...)
Be sure that the control flow of your code does not uses jumps (return, break, raise, ...)
Be sure that you don't perform any side effect.

Let's try to use these rules to solve your question.

Clean the code

The first thing to fix is the type of the return value. Depending on the situation, you return either a string or an integer. Althought Python does not enforce a function to type consistency, you should avoid such a confusion. You can either return a special value (-1) or raise an exception.

Here are a few more things to fix:

i is never used: replace it by _ (see What is the purpose of the single underscore "_" variable in Python?);
compute q.index(max(q)) once and store it in a variable max_q_index;
x in range(len(q)) means 0 <= x < len(q);
remove useless parentheses.

You get:

for _ in range(len(q)):
    m = max(q)
    max_q_index = q.index(m) # position of the max
    if max_q_index < len(q) - 3:
        return -1

    if max_q_index < len(q) - 2:
        bribe = bribe + 2
        q.remove(m)
    elif max_q_index < len(q) - 1:
        bribe = bribe + 1
        q.remove(m)
    elif max_q_index == len(q)-1:
        q.remove(m)
    # no else since max_index < len(q)

It's better, but it can be improved. You have actually four different cases:

max_q_index == len(q)-1
max_q_index == len(q)-2
max_q_index == len(q)-3
max_q_index < len(q)-3

You should replace the < to express this. In the three first cases, you remove the max from q:

for _ in range(len(q)):
    m = max(q)
    max_q_index = q.index(max(q)) # position of the max
    if max_index < len(q) - 3:
        return -1

    if max_index == len(q) - 3:
        bribe = bribe + 2
    elif max_index == len(q) - 2:
        bribe = bribe + 1
    q.remove(m)

Now we understand what's happening: if the list is almost sorted, the max is always in the three last items and the loop iterates over every element of q. Otherwise, you break. You could write it like that:

for _ in range(len(q)):
    m = max(q)
    max_q_index = q.index(m) # position of the max
    distance = len(q) - 1 - max_q_index
    if distance >= 3:
        return -1

    bribe += distance
    q.remove(m)

Should I use a list comprehension?

You want a sum of distances, thus the rule 2 is observed, but the control flow uses jumps: you return as soon as a distance >= 3 is found and you remove elements from q on every iteration (side effect). You should not use a list comprehension here.

Bonus

You can use a different strategy: sort the values along with their indices:

>>> L = [1,2,4,3,7,5]
>>> list(zip(L, range(len(L))))
[(1, 0), (2, 1), (4, 2), (3, 3), (7, 4), (5, 5)]
>>> S = sorted(zip(L, range(len(L))))
>>> S
[(1, 0), (2, 1), (3, 3), (4, 2), (5, 5), (7, 4)]

The function zip zips the elements of the list and the numbers 0,1,2,3,4..., ie their position in the list. We sort the produced tuples by value. Now, compare the second value of the tuples (position) with the current index of the tuple: if the position if lower or equal to the index, the max is not the last element of the list and we increase the bribe; if the position is greater than the index, the max is the righmost value of the list.

>>> bribe = 0
>>> for i, (_, pos) in enumerate(S):
...     distance = max(i - pos, 0)
...     if distance >= 3:
...         raise Exception() # can't return outside of a function.
...     bribe += distance
...
>>> bribe
2

With L = [1,2,7,3,4,5], you'll get an exception.

If you don't need the shortcut for distance >= 3 you can use a list comprehension:

>>> L = [1,2,4,3,7,5]
>>> sum(max(i - pos, 0) for i, (_, pos) in enumerate(sorted(zip(L, range(len(L))))))
2
>>> L = [1,2,7,3,4,5]
>>> sum(max(i - pos, 0) for i, (_, pos) in enumerate(sorted(zip(L, range(len(L))))))
3

It is possible to hack the list comprehension to throw an exception and meet the initial behaviour, but don't do it.

How to convert a for loop with multiple statements in if, elif conditions to List Comprehensions

1 Answers1

Clean the code

Should I use a list comprehension?

Bonus