HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

What is the best way to extract method?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thewhatmethodwayextractbest

Problem

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?

Solution

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)

Code Snippets

db_data = {}

def create_info(db_data, …):
    …
    return db_data

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

db_data = create_info(my_dict)
print(db_data)
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)
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)
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)

Context

StackExchange Code Review Q#127374, answer score: 4

Revisions (0)

No revisions yet.