patternpythonMinor
Filtering a list of citizens
Viewed 0 times
listcitizensfiltering
Problem
I have a filter method that receives a list composed of dictionaries. You must discard dictionaries that have their
Note that the list concatenation for the
handle or their citizen_number elements off the filtered result as well as concatenate any lists inside that dictionary. In addition to that, in case an element on that main list is a Noneit should be skipped and all the None dictionary elements should be replaced by a zero. This is the solution that I have come up with to do sodef citizens(citizen_list):
filtered_list = []
try:
for citizen in citizen_list:
if citizen:
for item in citizen:
if not item:
citizen[item] = 0
if (citizen['handle'] == 0 or citizen['citizen_number'] == 0):
continue
if (citizen['organizations'] is not None):
citizen['organizations'] = ', '.join(
org['sid'] for org in citizen['organizations'])
else:
citizen['organizations'] = 0
for item in citizen:
if(type(citizen[item]) is list):
citizen[item] = ', '.join(citizen[item])
filtered_list.append(citizen)
return(filtered_list)
except Exception:
print(citizen)
raiseNote that the list concatenation for the
organizationsfield must be done before the general one and in that particular way. I feel this code just looks awfully hacked together, how can I make this better?Solution
I can't speak to the specifics of the problem, since I don't understand exactly what you're trying to do, but the general idea seems to be:
To make that clearer, we can rewrite the code this way:
This has several benefits. It will reduce multiple layers of indentation. It will give you two functions with focused criteria. And it will make it clear that
- Filter
citizen_listbased on some critiera
- Apply some transformation to each citizen in
citizen_list
To make that clearer, we can rewrite the code this way:
def _transform_citizen(citizen):
# ???
def _is_valid_citizen(citizen):
return citizen['handle'] and citizen['citizen_number']
def citizens(citizen_list):
return [
_transform_citizen(citizen)
for citizen in citizen_list
if _is_valid_citizen(citizen)
]This has several benefits. It will reduce multiple layers of indentation. It will give you two functions with focused criteria. And it will make it clear that
citizens() is doing just a filter and a transformation on the input list.Code Snippets
def _transform_citizen(citizen):
# ???
def _is_valid_citizen(citizen):
return citizen['handle'] and citizen['citizen_number']
def citizens(citizen_list):
return [
_transform_citizen(citizen)
for citizen in citizen_list
if _is_valid_citizen(citizen)
]Context
StackExchange Code Review Q#113849, answer score: 8
Revisions (0)
No revisions yet.