4

I've got this block of code in a real Django function. If certain conditions are met, items are added to the list.

ret = []

if self.taken():
    ret.append('taken')

if self.suggested():
    ret.append('suggested')

#.... many more conditions and appends...

return ret

It's very functional. You know what it does, and that's great...
But I've learned to appreciate the beauty of list and dict comprehensions.

Is there a more Pythonic way of phrasing this construct, perhaps that initialises and populates the array in one blow?

Federico Zancan
  • 4,846
  • 4
  • 44
  • 60
Oli
  • 235,628
  • 64
  • 220
  • 299

7 Answers7

3

Create a mapping dictionary:

self.map_dict = {'taken': self.taken,
                 'suggested': self.suggested,
                 'foo' : self.bar}
[x for x in ['taken', 'suggested', 'foo'] if self.map_dict.get(x, lambda:False)()]

Related: Most efficient way of making an if-elif-elif-else statement when the else is done the most?

Community
  • 1
  • 1
Ashwini Chaudhary
  • 244,495
  • 58
  • 464
  • 504
2

Not a big improvement, but I'll mention it:

def populate():
    if self.taken():
        yield 'taken'
    if self.suggested():
        yield 'suggested'

ret = list(populate())

Can we do better? I'm skeptical. Clearly there's a need of using another syntax than a list literal, because we no longer have the "1 expression = 1 element in result" invariant.


Edit:

There's a pattern to our data, and it's a list of (condition, value) pairs. We might try to exploit it using:

[value
 for condition, value
 in [(self.taken(), 'taken'),
     (self.suggested(), 'suggested')]
 if condition]

but this still is a restriction for how you describe your logic, still has the nasty side effect of evaluating all values no matter the condition (unless you throw in a ton of lambdas), and I can't really see it as an improvement over what we've started with.

Kos
  • 70,399
  • 25
  • 169
  • 233
1

For this very specific example, I could do:

return [x for x in ['taken', 'suggested', ...] if getattr(self, x)()]

But again, this only works where the item and method it calls to check have the same name, ie for my exact code. It could be adapted but it's a bit crusty. I'm very open to other solutions!

Oli
  • 235,628
  • 64
  • 220
  • 299
1

I don't know why we are appending strings that match the function names, but if this is a general pattern, we can use that. Functions have a __name__ attribute and I think it always contains what you want in the list.

So how about:

return [fn.__name__ for fn in (self.taken, self.suggested, foo, bar, baz) if fn()]

If I understand the problem correctly, this works just as well for non-member functions as for member functions.

EDIT:

Okay, let's add a mapping dictionary. And split out the function names into a tuple or list.

fns_to_check = (self.taken, self.suggested, foo, bar, baz)

# This holds only the exceptions; if a function isn't in here,
# we will use the .__name__ attribute.
fn_name_map = {foo:'alternate', bar:'other'}

def fn_name(fn):
    """Return name from exceptions map, or .__name__ if not in map"""
    return fn_name_map.get(fn, fn.__name__)

return [fn_name(fn) for fn in fns_to_check if fn()]

You could also just use @hcwhsa's mapping dictionary answer. The main difference here is I'm suggesting just mapping the exceptions.

steveha
  • 74,789
  • 21
  • 92
  • 117
1

One option is to have a "sentinel"-style object to take the place of list entries that fail the corresponding condition. Then a function can be defined to filter out the missing items:

# "sentinel indicating a list element that should be skipped
Skip = object()

def drop_missing(itr):
    """returns an iterator yielding all but Skip objects from the given itr"""
    return filter(lambda v: v is not Skip, itr)

With this simple machinery, we come reasonably close to list-comprehension style syntax:

return drop_skips([
    'taken' if self.taken else Skip,
    'suggested' if self.suggested else Skip,
    100 if self.full else Skip,
    // many other values and conditions
])
teichert
  • 3,963
  • 1
  • 31
  • 37
0

In another instance (where a value will be defined but might be None - a Django model's fields in my case), I've found that just adding them and filtering works:

return filter(None, [self.user, self.partner])

If either of those is None, They'll be removed from the list. It's a little more intensive than just checking but still fairly easy way of cleaning the output without writing a book.

Oli
  • 235,628
  • 64
  • 220
  • 299
0
ret = [
    *('taken' for _i in range(1) if self.taken()),
    *('suggested' for _i in range(1) if self.suggested()),    
]

The idea is to use the list comprehension syntax to construct either a single element list with item 'taken', if self.taken() is True, or an empty list, if self.taken() is False, and then unpack it.

victorx
  • 3,267
  • 2
  • 25
  • 35