2
\$\begingroup\$

The code snippet is what I want to do. Previously I use a different version of my code. But it is out of joint according to Python coding guidelines because I was assigned my lambda function to a variable. It was an obvious violation. Here is my violator code:

my_dict = {
    'first_name': 'Jimmy Floyd',
    'last_name': 'Hasselbaink'
}

db_data = {}

def create_info(db_data, data):
    split_join = lambda field: '_'.join(data.get(field).split()) if field in data else None
    name = split_join('name')
    first_name = split_join('first_name')
    last_name = split_join('last_name')
    code = '_'.join(filter(None, [name, first_name, last_name])).lower()
    db_data['code'] = code
    return db_data

Then I have extracted my split_join variable to a method. The code is below:

my_dict = {
    'first_name': 'Jimmy Floyd',
    'last_name': 'Hasselbaink'
}

db_data = {}

def split_join(data, field):
    return '_'.join(data.get(field).split()) if field in data else None

def create_info(db_data, data):
    name = split_join(data, 'name')
    first_name = split_join(data, 'first_name')
    last_name = split_join(data, 'last_name')
    code = '_'.join(filter(None, [name, first_name, last_name])).lower()
    db_data['code'] = code
    return db_data

new_db_data = create_info(db_data, my_dict)
print(new_db_data)

I have forced into writing split_join method in three times. Is there any way to prevent this situation?

\$\endgroup\$
  • \$\begingroup\$ Just to check, you do know that data is not being copied or modified when you pass it to split_join, right? It's just a reference, so no extra data is created nor is the original modified. \$\endgroup\$ – SuperBiasedMan May 3 '16 at 13:24
2
\$\begingroup\$
db_data = {}

def create_info(db_data, …):
    …
    return db_data

new_db_data = create_info(db_data, …)

This feels plain wrong. Either your create_info function mutates data, or it generates some; but not both. Judging by the name, your function should create the dictionary it returns:

def create_info(data):
    …
    code = …
    return {'code': code}

db_data = create_info(my_dict)
print(db_data)

If you want to avoid passing data as a parameter to split_join and not use a lambda altogether, you can still define a nested function which will capture the needed variables:

def create_info(data):
    def split_join(field):
        return '_'.join(data.get(field).split()) if field in data else None
    …
    code = …
    return {'code': code}

db_data = create_info(my_dict)
print(db_data)

You can avoid calling split_join several times by using map:

def create_info(data):
    def split_join(field):
        return '_'.join(data.get(field).split()) if field in data else None
    name, first_name, last_name = map(split_join, ('name', 'first_name', 'last_name'))
    code = '_'.join(filter(None, [name, first_name, last_name])).lower()
    return {'code': code}

db_data = create_info(my_dict)
print(db_data)

What is interesting with that, is that you can feed the result of map directly to the filter:

def create_info(data):
    def split_join(field):
        return '_'.join(data.get(field).split()) if field in data else None

    code = '_'.join(
            filter(
                None,
                map(split_join, ('name', 'first_name', 'last_name'))
            )
    )
    return {'code': code.lower()}

db_data = create_info(my_dict)
print(db_data)

Last improvement would be to not rely on None being generated when there is no data available, but '' instead, since you’re only manipulating strings:

def create_info(data):
    def split_join(field):
        return '_'.join(data.get(field, '').split())

    code = '_'.join(
            filter(
                bool,
                map(split_join, ('name', 'first_name', 'last_name'))
            )
    )
    return {'code': code.lower()}

db_data = create_info(my_dict)
print(db_data)

Update According to your comment:

You can obviously remove the need for a nested function by switching back to a lambda:

def create_info(data):
    code = '_'.join(
            filter(
                bool,
                map(lambda field: '_'.join(data.get(field, '').split()),
                    ('name', 'first_name', 'last_name')
                )
            )
    )
    return {'code': code.lower()}

db_data = create_info(my_dict)
print(db_data)

But readability is becoming worse. In any case, it is often recommended to convert map + lambda into an explicit list-comprehension or generator expression instead. I’m choosing the generator expression here because we won't need an intermediate list:

def create_info(data):
    code = '_'.join(
            filter(
                bool, (
                    '_'.join(data.get(field, '').split())
                    for field in ('name', 'first_name', 'last_name')
                )
            )
    )
    return {'code': code.lower()}

db_data = create_info(my_dict)
print(db_data)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.