关注点分离:何时“太多”分离?

我真的很喜欢干净的代码,我总是希望以最好的方式编写代码。但总有一件事,我真的不明白:

关于方法的“关注点分离”是什么时候?

假设我们有以下方法:

def get_last_appearance_of_keyword(file, keyword):
    with open(file, 'r') as file:
        line_number = 0
        for line in file:
            if keyword in line:
                line_number = line
        return line_number

我认为这种方法很好。它的名字很简单,易读,而且很明显。但是:它并没有真正做到“只做一件事”。它实际打开文件,然后找到它。这意味着我可以进一步分裂它(同时考虑“单一责任原则”):

变化B(嗯,这在某种程度上是有道理的。通过这种方式,我们可以轻松地重复使用在文本中查找关键字的最后外观的算法,但它看起来像“太多”。我无法解释原因,但我只是“感觉到“就这样):

def get_last_appearance_of_keyword(file, keyword):
    with open(file, 'r') as text_from_file:
        line_number = find_last_appearance_of_keyword(text_from_file, keyword) 
    return line_number

def find_last_appearance_of_keyword(text, keyword):
    line_number = 0
    for line in text:
        if keyword in line:
            line_number = line
    return line_number

变化C(这在我看来是荒谬的。我们基本上只用一行两次将单行封装到另一个方法中。但有人可能会说,由于某些功能请求,打开某些东西的方式可能会在未来发生变化,因为我们不想多次改变它,但只是一次,我们只是封装它并进一步分离我们的主要功能):

def get_last_appearance_of_keyword(file, keyword):
    text_from_file = get_text_from_file(file)
    line_number = find_keyword_in_text(text_from_file, keyword)
    return line_number 

def get_text_from_file(file):
    with open(file, 'r') as text:
        return text

def find_last_appearance_of_keyword(text, keyword):
    line_number = 0
    for line in text:
        if check_if_keyword_in_string(line, keyword):
            line_number = line         
    return line_number

def check_if_keyword_in_string(text, keyword):
    if keyword in string:
        return true
    return false

所以现在我的问题是:编写此代码的正确方法是什么?为什么其他方法是对还是错?我总是学习:分离,但从来没有过多。我怎么能在将来确定它“恰到好处”并且当我再次编码时它不需要更多的分离?

6
额外 作者 gnat,
在上一个示例中,您重新实现了两个现有函数: open中的。重新实现现有功能并不会增加关注点的分离,现有功能已经解决了这个问题!
额外 作者 user35925,
旁白:你打算返回一个字符串或数字吗? line_number = 0 是一个数字默认值, line_number = line 指定一个字符串值(行内容而不是它的位置的)
额外 作者 Caleth,

6 答案

您将问题分解为单独函数的各种示例都会遇到同样的问题:您仍然将文件依赖项硬编码为 get_last_appearance_of_keyword 。这使得该功能难以测试,因为它现在必须在运行测试时回复文件系统中存在的文件。这导致脆弱的测试。

所以我只需将原来的功能更改为:

def get_last_appearance_of_keyword(text, keyword):
    line_number = 0
    for line in text:
        if keyword in line:
            line_number = line
    return line_number

现在你有一个只有一个职责的函数:在某些文本中找到最后一个关键字。如果该文本来自文件,那将成为调用者处理的责任。测试时,您可以传入一段文本。将其与运行时代码一起使用时,首先读取文件,然后调用此函数。这是真正的关注点分离。

6
额外
考虑不区分大小写的搜索。考虑跳过评论行。关注点的分离可能会有所不同。此外, line_number = line 显然是一个错误。
额外 作者 Rorick,
最后一个例子也是如此
额外 作者 Ewan,

什么时候“太多”分离?决不。你不能有太多的分离。

Your last example is pretty good, but you could maybe simplify the for loop with a text.GetLines(i=>i.containsKeyword) or something.

*实用版:工作时停止。当它破裂时分开更多。

4
额外
@cariehl你应该添加一个答案来争论这个案子。我想你会发现,实际上你需要在这些函数中使用更多的逻辑
额外 作者 Ewan,
额外 作者 Ewan,
“你不能有太多的分离。”我不认为这是真的。 OP的第三个例子就是将常见的python构造重写为单独的函数。我是否真的需要一个全新的功能才能执行'if x in y'?
额外 作者 Chthonic One,

我会说,确实没有太多的关注点分离。但是有些功能只能使用一次,甚至不能单独测试。它们可以安全地内联,保持与渗透到外部命名空间的分离。

您的示例字面上不需要 check_if_keyword_in_string ,因为字符串类已经提供了一个实现:行中的关键字就足够了。但您可能计划将实施交换为例如一个使用Boyer-Moore搜索,或允许在生成器中进行惰性搜索;那会有意义。

您的 find_last_appearance_of_keyword 可能更通用,并查找序列中项目的最后一个外观。为此,您可以使用现有实现,或进行可重用实现。此外,它可以采用不同的过滤器,以便您可以搜索正则表达式,或者不区分大小写的匹配项等。

通常任何处理I/O的东西都应该有一个单独的函数,所以如果你想彻底处理各种特殊情况, get_text_from_file 可能是个好主意。如果您依赖外部 IOError 处理程序,则可能不是这样。

如果将来你可能需要支持例如行计数可能是一个单独的问题。连续行(例如,使用 \ )并且需要逻辑行号。或者您可能需要忽略注释行,而不会破坏行号。

考虑:

def get_last_appearance_of_keyword(filename, keyword):
    with open(filename) as f:  # File-opening concern.
        numbered_lines = enumerate(f, start=1)  # Line-numbering concern.
        last_line = None  # Also a concern! Some e.g. prefer -1.
        for line_number, line in numbered_lines:  # The searching concern.
            if keyword in line: # The matching concern, applied.
                last_line = line_number
    # Here the file closes; an I/O concern again.
    return last_line

当您考虑将来可能会发生变化的某些问题时,或者仅仅因为您注意到在其他地方可以重复使用相同的代码时,请了解如何希望分割代码。

当你写出原始的简短功能时,需要注意这一点。即使您还不需要将问题分开作为函数,也要尽可能地将它们分开。它不仅有助于以后进化代码,还有助于更好地立即理解代码,减少错误。

2
额外

您遇到的问题是,您没有将功能分解为最简化的形式。看看下面的内容:(我不是一个python程序员,所以让我有些松懈)

def lines_from_file(file):
    with open(file, 'r') as text:
        line_number = 1
        lines = []
        for line in text:
            lines.append((line_number, line.strip()))
            line_number += 1
    return lines

def filter(l, func):
    new_l = []
    for x in l:
        if func(x):
            new_l.append(x)
    return new_l

def contains(needle):
    return lambda haystack: needle in haystack

def last(l):
    length = len(l)
    if length > 0:
        return l[length - 1]
    else:
        return None

上述每个功能都完全不同,我相信你会很难将这些功能分解出来。我们可以将这些功能结合起来完成手头的任务。

lines = lines_from_file('./test_file')
filtered = filter(lines, lambda x : contains('some value')(x[1]))
line = last(filtered)
if line is not None:
    print(line[0])

上面的代码行可以很容易地组合成一个函数,以完全执行您要执行的操作。真正区分问题的方法是将复杂的操作分解为最常见的形式。一旦你有一组很好的因子函数,你可以开始拼凑它们来解决更复杂的问题。关于良好分解函数的一个好处是它们通常可以在当前问题的上下文之外重用。

1
额外

单一责任原则规定一个类应该处理单一功能,并且该功能应该适当地封装在内部。

你的方法究竟做了什么?它获得了关键字的最后一次出现。方法中的每一行都适用于此,它与其他任何东西无关,最终结果只有一个和一个。换句话说:您不必将此方法拆分为其他任何方法。

这个原则背后的主要思想是你最终不应该做多件事。也许你打开文件并保留它,所以其他方法可以使用它,你将做两件事。或者,如果你要坚持与这种方法相关的数据,还有两件事。

现在,您可以提取“打开文件”行并使方法接收要使用的文件对象,但这比尝试遵守SRP更具技术性重构。

这是过度工程的一个很好的例子。不要想太多,否则你最终会得到一堆单行方法。

1
额外
@JoshuaJones单行函数没有固有的错误,但如果它们没有抽象出任何有用的东西,它们可能是一个障碍。用于返回两点之间的笛卡尔距离的单线函数非常有用,但是如果在文本中有 return keyword的单行,那么只需在构建的顶部添加一个不必要的层。在语言结构中。
额外 作者 Chthonic One,
@JoshuaJones在这种情况下,你抽象出一些有用的东西。在原始示例的上下文中,没有充分的理由存在这样的函数。 中的是一个常见的Python关键字,它实现了目标,并且它本身就具有表现力。围绕它编写包装函数只是为了使包装函数模糊代码,使其不那么直观。
额外 作者 Chthonic One,
单线功能绝对没有问题。实际上,一些最有用的函数只是一行代码。
额外 作者 dfdffewfw,
@cariehl为什么在文本中返回关键字是不必要的层?如果您发现自己一直在lambda中使用该代码作为高阶函数中的参数,为什么不将它包装在函数中呢?
额外 作者 dfdffewfw,

我接受它:这取决于:-)

在我看来,代码应该达到这个目标,按优先级排序:

  1. 满足所有要求(即它正确地做了它应该做的事)
  2. 易读且易于理解
  3. 易于重构
  4. 遵循良好的编码惯例/原则

对我来说,你的原始示例传递了所有这些目标(除了正确性,因为 line_number = line /分离关注 - 什么时候太多分离#comment833656_378914“>评论,但这不是重点。

问题是,SRP不是唯一遵循的原则。还有你不需要它(YAGNI)(以及其他许多人)。当原则发生冲突时,你需要平衡它们。

您的第一个示例是完全可读的,在您需要时易于重构,但可能不会尽可能多地遵循SRP。

您的第三个示例中的每个方法也都是完全可读的,但整个过程不再那么容易理解,因为您必须将所有部分插入到您的脑海中。它确实遵循SRP。

由于您没有从分割方法中获得任何 ,所以不要这样做,因为您有一个更容易理解的替代方法。

随着您的要求发生变化,您可以相应地重构该方法。事实上,“一体化”可能更容易重构:想象一下,你想要找到符合某些任意标准的最后一行。现在你只需要传递一些谓词lambda函数来评估一条线是否符合标准。

def get_last_match(file, predicate):
    with open(file, 'r') as file:
        line_number = 0
        for line in file:
            if predicate matches line:
                line_number = line
        return line_number

在上一个示例中,您需要深度传递谓词3级,即修改3个方法只是为了修改最后一个的行为。

请注意,即使拆分文件的读取(通常很多看起来有用的重构,包括我)也可能会产生意想不到的后果:您需要将整个文件读取到内存中以将其作为字符串传递给您的方法。如果文件很大,可能不是您想要的。

一句话:在不退后一步并考虑所有其他因素的情况下,原则绝不应该走到极致。

也许“过早分裂方法”可能被视为过早优化的特例? ;-)

0
额外