There are several possible improvements. First, I'd like to mention some points that pertain to the general style of your code. Then, I'm going to discuss some aspects of your code that are unnecessarily complex. I'll then show my alternative to your code, followed by a discussion of why I think your required output may not be optimal.
Formating and style
Variable naming: input_data
, respo
, and i
are all names that don't really help understanding your code. i
is particularly misleading as it's a name that's usually used for integer variables e.g. in indexes or for
loops. Instead, I'd suggest to use more telling names. Of course, this depends on the intended use of your code, which you didn't specify though.
You use both single and double quotation marks to delimit strings. Either one is acceptable, but you should be consistent.
More generally, your code formatting can be improved by paying attention to the Python style guide as described in PEP 8. In addition the first two points I made, it also discusses issues such as the use of whitespace after e.g. the key separator :
as well as a suggested maximu line length of 79 characters.
Code
If you want to check whether a dictionary contains a certain key, you don't need to call the keys()
method. A simple test like if key in dct:
is already sufficient.
The use of get()
for every dictionary access impairs the the readability of your code. The primary purpose of this method is that it doesn't raise a KeyError
if the key doesn't exist, but returns a default value (None
unless specified otherwise). This can be useful if your input data isn't always well-formed (e.g. if one of the elements may contains key-value pairs for name
and region
, but not for account_type
or currency
). However, for your example data, that's not really the case, so there's nothing to be gained from using the less explicit get()
.
Many of your if
instructions are unnecessary, at least for the input data that you've provided. For instance, every element in your input list has a region
field. Hence, it's guaranteed that the dictionaries stored in your output structure will also contain the region code. But still, your code contains an explicit test for that, i.e. if i.get("region") in respo[i.get("name")].keys()
. If you can trust that your input data will always be complete, you can leave out this if
condition.
Related to the previous point, you may want to use defaultdict
from the collections
module. This data type is an expansion of the dict
type: if you try to access an element using a non-existing key, the interpreter doesn't raise a KeyValue
exception but automatically creates a default element for that key. For details, please consult the documentation.
Suggested alternative
Here is my suggested alternative for your code. It conforms to the PEP 8 coding style guidelines by using consistent string quote characters, more useful names, proper whitespace and appropriate overall formatting. It also makes the handling of your list and dictionaries more explicit by (a) storing values that are used more than once in variables and (b) by using defaultdict
where appropriate.
from collections import defaultdict
customers = {}
for account in input_data:
name = account["name"]
region = account["region"]
account_type = account["type"]
currency = account["currency"]
if name not in customers:
customers[name] = {"name": name, region: defaultdict(list)}
customers[name][region][account_type].append(currency)
Comment on expected output
I don't know how you're going to use the output structure (respo
in your question, and customers
in my solution), but there's one thing that I noticed: The field name
in the dictionaries that are stored in your output variable stores the same string that is used as a key to access that dictionary. This redundancy is something of a code smell: typically, if you can access one of the dictionaries stored in respo
, you already know the value of name
, so why store it? Now you'll have to make sure that whenever either name
or the key in respo
changes, the other value changes as well.
It also makes handling the data contained in the dictionary more complicated. For instance, if you want to process all customers in a tabular form, you'll always have to include a special case that filters out the name
key:
print(f"{'NAME':15} {'REGION':15} {'TYPE':15} CURRENCIES")
for name in customers:
for key in customers[name]:
# the next if condition is only needed due to the redundant key:
if key != "name":
for account_type in customers[name][key]:
currencies = customers[name][region][account_type]
print(f"{name:15} {region:15} {account_type:15} "
f"{', '.join(currencies)}")
If you don't store the name in customers
again, i.e. if there was no name
key for each dictionary stored in customers
, you could easily change the transformation code so that customers
is also a defaultdict
, so my solution could be simplified even more, like so:
from collections import defaultdict
customers = defaultdict(lambda: defaultdict(lambda: defaultdict(list)))
for account in input_data:
name = account["name"]
region = account["region"]
account_type = account["type"]
currency = account["currency"]
customers[name][region][account_type].append(currency)
print(f"{'NAME':15} {'REGION':15} {'TYPE':15} CURRENCIES")
for name in customers:
for region in customers[name]:
for account_type in customers[name][region]:
currencies = customers[name][region][account_type]
print(f"{name:15} {region:15} {account_type:15} "
f"{', '.join(currencies)}")
As you can see, both generating the structure as well as traversing through it becomes almost trivial now.