1

How do you think of two pieces of code snippets in Python?

for a in aa:
    if a in nb:
        intsct.append(a)
        nb.remove(a)

what if the code snippet over is rewritten as following?

[intsct.append(a) or nb.remove(a) for a in aa if a in nb]

Although both of them gives me the same result, which one is better in terms of code practices in Python? If compactness is emphasized, can I go for the second one?

Sam D.
  • 265
  • 2
  • 10
  • 4
    Both fail the crucial test: descriptive variable names. – roganjosh Nov 01 '18 at 16:19
  • 1
    In general a list comprehension is considered more "Pythonic" than a loop. ("Flat is better than nested" is part of the "Zen of Python".) BUT it's not clear to me that these two pieces of code to the same thing at all. Why has an `or` operator appeared in the comprehension? And it's very strange (to my eyes) to use a list comprehension to simply do an operation on each element of a list, as opposed to creating a new list from the original. – Robin Zigmond Nov 01 '18 at 16:20
  • 6
    Voting to close as primarily opinion-based. But the second code, in my view, obscures the intent and generates a list of `None` for no reason. This isn't an appropriate use of a list comprehension, assuming it works. There's almost certainly a better approach than either of these (likely using a `set`); if you [posted your original problem](https://en.wikipedia.org/wiki/XY_problem) it'd be clearer. – ggorlen Nov 01 '18 at 16:20
  • Also, you're removing items from `nb` while iterating over it. That's usually not a good idea. – roganjosh Nov 01 '18 at 16:21
  • @roganjosh not quite the case; iterating is done over `aa` so that seems safe. – ggorlen Nov 01 '18 at 16:22
  • @ggorlen oops, misread. My point about names stand :) – roganjosh Nov 01 '18 at 16:22
  • 1
    Does the list comprehension even run? I've never seen `or` used in the beginning like that. – Charles Landau Nov 01 '18 at 16:22
  • Also, the setup of the list comprehension is a _side effect_ here because you're using it to drive a `for` loop that actually populates a different list. – roganjosh Nov 01 '18 at 16:23
  • 1
    The list comprehension works, but it's an abuse of both `or` and the list comprehension itself. – tobias_k Nov 01 '18 at 16:23
  • @CharlesLandau: `append` returns `None` so it always does both. – Jongware Nov 01 '18 at 16:26
  • I disagree that this is "primarily opinion-based". They're asking about Python best practices and _why_ one approach is more Pythonic than the other. I think that's a reasonable question. – Kirk Strauser Nov 01 '18 at 16:26
  • 2
    @RobinZigmond: List comprehensions are Pythonic when the goal is producing a brand new `list` from an existing iterable, with no side-effects. If you're using them for side-effects, or otherwise ignoring the resulting `list`, nope, not at all Pythonic. – ShadowRanger Nov 01 '18 at 16:26
  • @ShadowRanger agreed, I did say it looked very strange here. I started off with "in general" before moving on to mention some problems with this particular case. – Robin Zigmond Nov 01 '18 at 16:28
  • Based on the code operations, it looks like he would be better off using "set" operations. – Jay Atkinson Nov 01 '18 at 16:28
  • @usr2564301 Ah, so it results in a list of `None`s and as joshrogan said the comprehension is a side effect. Thanks! – Charles Landau Nov 01 '18 at 16:28
  • How about this? `intsct = [a for a in aa if a in nb]` and then `nb = [a for a in bn if a not in intsct]`? Of course, this creates new lists instead of appending to the old ones, but maybe this works for you, too. Or if you use `set` for `aa` and `nb`, just do `intsct = aa & nb` and `nb -= aa`. – tobias_k Nov 01 '18 at 16:31

1 Answers1

3

I don't like the second, because you're not actually using those results: you're building a list purely for its side effects, then throwing it away (which is also inefficient; imagine that aa has a billion items). It also depends on knowing that list.append returns None so that the or clause causes the list.remove to always be called.

Compactness is great when you can find an expressive way of writing your intent. I think this is terseness simply for the sake of terseness, and doesn't actually express the goal of the original code. Put another way, if you were my coworker and I was trying to figure out why this had broken at 2AM, I would be cursing your name.

Kirk Strauser
  • 30,189
  • 5
  • 49
  • 65