2

I have code that looks like this:

def transform_incoming_json_item(item, things=[]):
    if isinstance(item, list):
        for thing in item:
            things.append(process_thing(thing))
    elif isinstance(item, dict):
        things.append(process_thing(item))
    return things

Is there a more pythonic way of doing this while making only a single call to process_thing (while preserving the type check and the two cases, loop vs. no loop needed)?

(Note: "more pythonic" added in edit to clarify question.)

scharfmn
  • 3,561
  • 7
  • 38
  • 53
  • 2
    You need to watch out with `things` using `[]` as a default. That default is stored **once** with the function, not created anew each time you call the function. See ["Least Astonishment" in Python: The Mutable Default Argument](http://stackoverflow.com/q/1132941) – Martijn Pieters Feb 14 '15 at 07:56

2 Answers2

2

How about: for dict case, convert the dict to a list with a single item (the dict object):

def transform_incoming_json_item(item, things=[]):
    if isinstance(item, dict):    # <----
        item = [item]             # <----
    if isinstance(item, list):
        things.extend(process_thing(thing) for thing in item)
    return things

NOTE: the parameter things is created in definition time (only once). In other words, things is not initialized when the function transform_incoming_json_item is called.

If that's not what you want, you need to do following way:

def transform_incoming_json_item(item, things=None):
    if things is None:
        things = []
    if isinstance(item, dict):    # <----
        item = [item]             # <----
    if isinstance(item, list):
        things.extend(process_thing(thing) for thing in item)
    return things
falsetru
  • 357,413
  • 63
  • 732
  • 636
1

Using falsetru's idea of converting the dict to a list, you can further simplify the code to this:

def transform_incoming_json_item(item, things=None):
    return ([] if things is None else things +
        [process_thing(thing) for thing in (
         item  if isinstance(item, list) else 
        [item] if isinstance(item, dict) else [])])

As you can see, it's fairly compact. However, if list comprehensions this long confuse you or aren't your thing, you should obviously go with the other answer. Personally, I find them excellent for situations like this, but YMMV.

Eithos
  • 2,421
  • 13
  • 13
  • Doesn't confuse me, exactly, but it does help clarify that 'pythonic' does not always mean 'easily maintained'. Edited question to require 'more pythonic' rather than just 'pythonic'. – scharfmn Feb 14 '15 at 09:44
  • Actually, I think falsetru's answer is simpler and more readable than yours :P – Maciej Gol Feb 14 '15 at 09:58
  • @MaciejGol definitely nicer to look at, but is type shifting 'more pythonic'? – scharfmn Feb 14 '15 at 10:58
  • @bahmait, by type shifting you mean this `item = [item]`? – Maciej Gol Feb 14 '15 at 11:05
  • @MaciejGol yes - 'the change from dict to list' is what I mean - even though it is on its own line, I'm not sure that's easier to read, or 'more pythonic' – scharfmn Feb 14 '15 at 11:13
  • 1
    @bahmait, I wouldn't call it pythonic as I find it applies to any language. The point is that with this trick, you reduce the number of edge cases and reuse algorithm that is proven to be working for lists. It can't be applied in all cases, but here it works like a charm. You need to apply `process_thing` over each element of `item`. You can reduce the case where `item` is a dict to a case of 1-element list, and everything will work fine. – Maciej Gol Feb 14 '15 at 14:21
  • @MaciejGol I'm asking in a language specific sense: for example `try/except` is a very 'pythonic' way to test what kind of object one is dealing with, whereas it's heresy in other languages - I'm basically trying to get a sense of maintainability for this case (i.e. what other people want to see), because I find myself dealing with a lot of mystery json etc. – scharfmn Feb 14 '15 at 15:16