正是这段代码让我有这样的感觉:
def process_data(data, flag1, flag2, flag3):
if flag1:
data = [item for item in data if item.isdigit()]
if flag2:
data = [item.upper() for item in data]
if flag3:
data = sorted(data)
result = "Processed: " + ", ".join(data)
return result
乍一看,这段代码似乎没什么不好。但让我告诉你为什么我说这段代码很糟糕:
1. flag太多
它使用太多flag来控制行为,这使得功能有点混乱。
在此,每次我们调用一个函数时,我们都必须弄清楚哪种标志组合产生了所需的结果。
让我用这个例子来解释一下
res = process_data(my_data, True, False, True)
你明白True, False, Trueeven 是什么意思了吗? 和上面的函数是一样的。
2.缺乏单一职责
该函数试图同时执行多项操作,例如_过滤数字、将文本转换为大写、对数据进行排序以及将数据合并为字符串。_
我们可以为每个函数编写单独的函数。因为单一职责函数更易于阅读、测试和重用。
3. 缺少类型注解
在这个函数中,什么都没有解释。它是什么类型的数据?是列表、字符串还是字典?
我们必须猜测一下。
4. 难以扩展
例如,如果我们想添加另一个操作,如修剪空格或任何其他操作,那么我们必须添加另一个标志。
这会让功能变得更加混乱。
我如何修复此功能
我尝试以一种清晰、简单的方式重写该函数。
1. 我使用描述性的函数名称
我没有为每个任务使用标志,而是创建了这样的单独函数:
def filter_digits(data):
return [item for item in data if item.isdigit()]
def to_uppercase(data):
return [item.upper() for item in data]
def sort_data(data):
return sorted(data)
def format_result(data):
return "Processed: " + ", ".join(data)
现在每个功能只做一件事。
2. 创建一个管道
然后我尝试使用类似管道的方法将这些函数链接在一起。这样会更清晰。
def process_data_pipeline(data):
data = filter_digits(data)
data = to_uppercase(data)
data = sort_data(data)
return format_result(data)
3. 我让它更加灵活
我们可以使用函数列表来动态应用。如果我们想跳过一些步骤,这将很有用。
def process_data(data, steps):
for step in steps:
data = step(data)
return data
steps = [filter_digits, to_uppercase, sort_data, format_result]
result = process_data(my_data, steps)
现在,我们可以完全控制这些步骤。我们可以在不使用标志的情况下包含或排除这些步骤。
4. 添加类型注释
我只是添加了类型注释,以使该代码更具可读性且不易出错。
from typing import List, Callable
def process_data(data: List[str], steps: List[Callable[[List[str]], List[str]]]) -> str:
for step in steps:
data = step(data)
return data
为什么这种方式更好
- 每个函数现在都有明确定义的职责,这提高了代码的可读性
- 当我们需要在其他地方过滤数字时,可以直接调用filter_digits函数,这增强了代码的可重用性
- 测试变得更加简单。测试单一功能函数比测试臃肿的多功能函数容易得多
- 可扩展性显著提升。如果需要添加新步骤,只需编写新函数并添加到流程中即可
因此,下次编写函数时请自问这些问题:
- 这个函数是否承担了过多职责?其他开发者能否无需详细解释就能理解这段代码?
- 如果答案是否定的,你就需要重构代码了
你是否遇到过类似的情况?欢迎在评论区分享你的经历,我很期待听到大家的实践经验。