Making a complicated list comprehension conditional

Question

Here is my current one-liner:

leader = [server.get_member(x) for x in self.rosters[server.id][clan]['members'] if discord.utils.get(server.get_member(x).roles, id="463226598351699968")]

I want to only run this if server.get_member(x) is not False. How can I add this extra logic into this list comprehension? I understand how to do a basic for in statement, but nesting it deeper than that becomes a bit confusing for me.

please translate this in to a normal `for loop` for readability and for your own sake if you are the one who has to maintain this for the forseeable future — gold_cy, Feb 15 '19 at 20:32
You should strive to write clean, maintainable code, not to cram everything into a single list-comprehension — juanpa.arrivillaga, Feb 15 '19 at 20:34
Python list comprehensions are great for a lot of things but if you start getting lost in the logic its probably time to break it into a regular loop — nathan.medz, Feb 15 '19 at 20:35
Is this your API or someone else's - can you improve the decomposition? It might be better to have `server.get_member(x)` return `None` if there aren't any members. If `self.rosters[server.id][clan]['members']` is your own code, then decompose it better into something more OO. — smci, Feb 15 '19 at 21:11
One strong code smell that this requires for-loop/if-statements rather than a list-comprehension is that you're working around a clear need to have to **store state** by calling `server.get_member(x)` twice, as @blue_note said. — smci, Feb 15 '19 at 22:06

Olivier Melançon · Answer 1 · 2019-02-15T20:54:59.407

In general, do not sacrifice readability for the sake of writing a one-liner. If it not immediately obvious how to do it with a list-comprehension, then use a for-loop.

leader = []

for x in self.rosters[server.id][clan]['members']:
    member = server.get_member(x)
    if member and discord.utils.get(member.roles, id="463226598351699968"):
        leader.append(member)

Although, in this specific case, since you do not need x, you can use map to apply server.get_member while iterating.

leader = [m for m in map(server.get_member, self.rosters[server.id][clan]['members'])
          if m and discord.utils.get(m.roles, id="463226598351699968")]

score 2 · Answer 2 · answered Feb 15 '19 at 20:41

2

You can't. The item in list comprehension can't be saved, so you'll have to evaluate it twice. Even if you could, don't. List comprehensions are for filtering, not for running code as side effect. It's unreadable and prone to mistakes.

answered Feb 15 '19 at 20:41

blue_note

27,712
9
72
90

blhsing · Answer 3 · 2019-02-15T20:51:39.567

In general, you can achieve the effect of a temporary variable assignment with a nested list comprehension that iterates through a 1-tuple:

leader = [m for x in self.rosters[server.id][clan]['members'] for m in (server.get_member(x),) if m and discord.utils.get(m.roles, id="463226598351699968")]

But in this particular case, as @OlivierMelançon pointed out in the comment, since the additional assignment is simply mapping a value to a function call, you can achieve the desired result with the map function instead:

leader = [m for m in map(server.get_member, self.rosters[server.id][clan]['members']) if m and discord.utils.get(m.roles, id="463226598351699968")]

score -1 · Answer 4 · answered Feb 15 '19 at 20:37

-1

While I agree with the comments suggesting you should not write this as a comprehension for readability you could try:

leader = [server.get_member(x) for x in self.rosters[server.id][clan]['members'] if discord.utils.get(server.get_member(x).roles, id="463226598351699968") if server.get_member(x)]

Making a complicated list comprehension conditional

4 Answers4