How to rewrite this as list comprehension?

[Alex Martelli]

"Serge Beaumont" <serge at sergebeaumont.com> wrote in message
news:91nj2q$s3p$1 at buty.wanadoo.nl...
> Hello,
>
> I'm trying to get my head around comprehensions. How would I rewrite this
> method?
>
>   def deadThings(self, type = None):
>     deadThings = self.destroyed[:]
>     for thing in self.things:
>       if thing.destroyed:
>         if type:
>           if isinstance(thing, type):
>             deadThings.append(thing)
>         else:
>           deadThings.append(thing)
>     return deadThings

Writing this as a _single_ list comprehension will likely
put 'too much fat on the fire':

def deadThings1(self,type=None):
  return [ thing for thing in self.destroyed+self.things
      if thing in self.destroyed or thing.destroyed and ... ]

this would be a seriously unnatural approach -- if you
want to return the concatenation of an existing list with
some subset of another one, expressing it as a concatenation
(of the existing list with a comprehension) is a serious
readability win over expressing it as one huge comprehension:

def deadThings2(self,type=None):
  return self.destroyed + [ thing for thing in self.things
      if thing.destroyed and ... ]

(besides, the form used in deadThings1 would have
different semantics if repetitions are allowed in
the lists, AND would be _seriously_ slower too!).

So the only remaining problem is how to express the
'...' part.  My preference would be for a local
named function -- names sometimes clarify things
a lot...:

def deadThings3(self,type=None):
    if type:
        def isDestroyed(thing, type=type):
            return thing.destroyed and isinstance(thing,type)
    else:
        def isDestroyed(thing):
            return thing.destroyed
    return self.destroyed + [ thing for thing in self.things
        if isDestroyed(thing) ]

I'd rather test 'if type' once, choosing the appropriate
selection function, than test it every time through the
loop, given it's not changing inside the loop anyway; not
so much an issue of performance, but of clarity and
simplicity.  But if you disagree, it's easy to put the
test for type in a single function rather than use it
to choose the right function:

def deadThings4(self,type=None):
    def isDestroyed(thing, type=type):
        return thing.destroyed and (
            not type or isinstance(thing,type))
    return self.destroyed + [ thing for thing in self.things
        if isDestroyed(thing) ]

If you're allergic to local named function, you _can_
expand this form if 'isDestroyed' right in the list
comprehension, though I'd say this makes things a
bit more obscured (an epsilon of clarity may be gained
through the idiom 'if X if Y or Z' rather than
'if X and (Y or Z)', though that's debatable...):

def deadThings5(self,type=None):
    return self.destroyed + [ thing for thing in self.things
        if thing.destroyed if not type or isinstance(type,thing)]

Fanatics of conciseness at all costs may prefer this
last form (which CAN be considered a single-liner, if
your terminal has long-enough lines), but my personal
tastes favour form number 3, above, or some variation
emphasizing slightly different factoring:

def deadThings6(self,type=None):
    if type:
        def inType(thing, type=type):
            return isinstance(thing,type)
    else:
        def inType(thing):
            return 1
    return self.destroyed + [ thing for thing in self.things
        if thing.destroyed and inType(thing) ]

(which I find clearer, but that's just me), or:

def deadThings7(self,type=None):
    def inType(thing, type=type):
        return not type or isinstance(thing,type)
    return self.destroyed + [ thing for thing in self.things
        if thing.destroyed and inType(thing) ]

which, I suspect, would be the first form to 'flow off
the keyboard' for most of us:-).


Alex