Filtering a list of citizens

Question

I have a filter method that receives a list composed of dictionaries. You must discard dictionaries that have their 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 so

def 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)
        raise

Note 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?

Answer 1

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:

1. Filter citizen_list based on some critiera
2. 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.