patternpythonMinor
What is the best way to extract method?
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:
Then I have extracted my split_join variable to a method. The code is below:
I have forced into writing split_join method in three times. Is there any way to prevent this situation?
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_dataThen 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.