什么时候函数太长？ [关闭]答案

【问题标题】：When is a function too long? [closed]什么时候函数太长？ [关闭]
【发布时间】：2010-10-03 07:19:10
【问题描述】：

35 行、55 行、100 行、300 行？你应该什么时候开始把它分开？我之所以问，是因为我有一个 60 行（包括 cmets）的函数，并且正在考虑将其拆分。

long_function(){ ... }

进入：

small_function_1(){...}
small_function_2(){...}
small_function_3(){...}

这些函数不会在 long_function 之外使用，制作更小的函数意味着更多的函数调用，等等。

你什么时候会把一个函数分解成更小的函数？为什么？

方法应该只做一件合乎逻辑的事情（考虑功能）
你应该能用一句话解释方法
它应该适合显示器的高度
避免不必要的开销（cmets 指出显而易见的......）
小逻辑函数的单元测试更容易
检查部分函数是否可以被其他类或方法重用
避免过度的类间耦合
避免深度嵌套的控制结构

感谢大家的回答，编辑列表并投票给正确的答案，我会选择那个；）

考虑到这些想法，我现在正在重构 :)

【问题讨论】：

您在代码行方面提出问题是错误的。决定因素不是用代码行来衡量的。
这个问题可能会变得复杂，具体取决于代码和语言。也许你可以发布它。
如果符合单一责任原则，那就去做吧。我通常觉得需要为每 20 行代码创建一个标题，这标志着我将其抽象出来并将这个片段命名为一个有意义的名称的函数，而不是创建一个章节标题。
What is the ideal length of a method for you?

标签： function refactoring coding-style

【解决方案1】：

这部分是个人喜好问题，但我如何确定这是我尽量只保留我的功能，只要一次适合我的屏幕（最多）。原因是如果您能立即看到整个事情，就更容易理解正在发生的事情。

当我编写代码时，会先编写长函数，然后进行重构以提取可被其他函数重用的位，以及编写执行离散任务的小函数。

我不知道对此有任何正确或错误的答案（例如，您可能会选择 67 行作为您的最大值，但有时可能需要添加更多行）。

【讨论】：

好吧，我也喜欢在屏幕上看到我的完整功能 :) 有时这意味着 Monospace 9 字体和黑色背景中的大分辨率，我同意这样更容易理解。跨度>

【解决方案2】：

我通常分解一个函数的主要原因要么是因为它的点点滴滴也是我正在编写的另一个附近函数的成分，所以公共部分被排除在外。此外，如果它使用了其他类中的大量字段或属性，则很有可能可以将相关块大量提取出来，并在可能的情况下移入其他类。

如果您的代码块顶部带有注释，请考虑将其拉出到函数中，并用函数和参数名称说明其用途，并保留注释以说明代码的基本原理。

你确定那里没有在其他地方有用的部件吗？它是一个什么样的功能？

【讨论】：

该函数根据 url 从模板生成缓存文件，例如来自 url /post/2009/01/01 的 post_2009_01_01.html 感谢您的回答

【解决方案3】：

一个函数应该只做一件事。如果你在一个函数中做很多小事，把每一个小事都变成一个函数，然后从长函数中调用这些函数。

您真正不想想要做的是将长函数的每 10 行复制并粘贴到短函数中（如您的示例所示）。

【讨论】：

是的，用复制粘贴模式做很多小功能不是一个好主意，我同意一个功能应该总是只做一件事
“做一件事”可能正确也可能不正确，具体取决于粒度。如果一个函数乘以一个矩阵，那很好。如果一个函数构建了一辆虚拟汽车——那是“一件事”，但它也是一件非常大的事情。多个功能可用于构建汽车，逐个组件。

【解决方案4】：

没有真正的硬性规定。一般来说，我喜欢我的方法只是“做一件事”。因此，如果它正在抓取数据，然后用该数据做某事，然后将其写入磁盘，那么我会将抓取和写入拆分为单独的方法，因此我的“主要”方法只包含“做某事”。

虽然“做某事”可能仍然有很多行，所以我不确定使用多少行是正确的指标:)

编辑：这是我上周在工作中邮寄的一行代码（为了证明一点......这不是我养成的习惯:)） - 我当然不想要这些坏男孩中的 50-60 个在我的方法中：D

return level4 != null ? GetResources().Where(r => (r.Level2 == (int)level2) && (r.Level3 == (int)level3) && (r.Level4 == (int)level4)).ToList() : level3 != null ? GetResources().Where(r => (r.Level2 == (int)level2) && (r.Level3 == (int)level3)).ToList() : level2 != null ? GetResources().Where(r => (r.Level2 == (int)level2)).ToList() : GetAllResourceList();

【讨论】：

LOL 好吧，我可以删除我方法中的所有空格，它只会是一个很长的行而不是一个很长的函数。做一件事，这可能就是答案，谢谢
@Movaxes 我发布的代码 sn-p 是一个声明，而不仅仅是一行上的很多行.. 那里没有分号:) 我可以扩展 GetResources()每次都让它变得更加邪恶：P
是的，这是有道理的。为什么不把整个源文件放在一行中。我的意思是你真的可以成为 Web 2.0 的“忍者”:)
我记得在旧杂志（我说的是 BBC Micro old）中，他们曾经有“10 行节目”，每行只有几个语句，直到 BBC 可以处理的最大长度.. 输入它们总是很痛苦：D
我喜欢函数只做一件事的概念，....但是。如果您有一个执行 10 件事情的函数，并且您将其中的 9 件事情移动到单独的函数中，那么仍然由剩余函数调用的不是剩余函数本质上仍在做 10 件事情！我确实认为像这样分解函数会更容易测试。

【解决方案5】：

我怀疑你会在这方面找到很多答案。

我可能会根据函数中正在执行的逻辑任务将其分解。如果您认为您的短篇小说正在变成小说，我建议您找到并提取不同的步骤。

例如，如果您有一个处理某种字符串输入并返回字符串结果的函数，您可能会根据将字符串拆分为多个部分的逻辑、添加额外字符的逻辑和逻辑来分解函数将它们重新组合在一起作为格式化的结果。

简而言之，无论是让您的代码简洁易读（无论是通过简单地确保您的函数具有良好的注释还是将其分解）都是最好的方法。

【讨论】：

【解决方案6】：

60 行对于一个函数来说很大但不会太长。如果它适合编辑器的一个屏幕，您可以一次看到所有内容。这真的取决于函数在做什么。

为什么我可以分解一个函数：

太长了
通过分解代码并为新函数使用有意义的名称，使代码更易于维护
函数没有凝聚力
部分函数本身很有用。
很难为函数想出一个有意义的名称时（可能做得太多）

【讨论】：

你和这位伙伴的关系太差了。 60行总是太多了。我会说，如果你接近 10 行，你可能已经接近极限了。
但是另一个函数仍在调用这些函数，并且本质上是相同的 DoThisAndThisAndAlsoThis 函数，但有很多抽象，你仍然必须以某种方式命名

【解决方案7】：

在我看来，答案是：当它做太多事情时。您的函数应该只执行您期望从函数本身的名称中获得的操作。另一件要考虑的事情是，如果您想在其他人中重用您的某些部分功能；在这种情况下，拆分它可能很有用。

【讨论】：

【解决方案8】：

假设你正在做一个的事情，长度将取决于：

你在做什么
您使用的是什么语言
您需要在代码中处理多少层抽象

60 行可能太长，也可能恰到好处。我怀疑它可能太长了。

【讨论】：

【解决方案9】：

我通常根据需要放置描述下一个代码块的 cmets 来分解函数。以前进入 cmets 的内容现在进入新的函数名。这不是硬性规则，但（对我来说）一个很好的经验法则。比起需要 cmets 的代码，我更喜欢自己说话的代码（因为我知道 cmets 通常会撒谎）

【讨论】：

我喜欢评论我的代码，主要不是为了我，而是为了其他人，这消除了很多关于 $variable 定义的问题，但我也喜欢代码是自我解释的。 cmets会撒谎吗？
是的，因为它们通常不被维护。在撰写本文时，它们可能是正确的，但是一旦引入了错误修复或新功能，没有人会根据新情况强制更改 cmets。方法名称的谎言往往比 cmets 恕我直言
我刚刚遇到了这个答案：stackoverflow.com/questions/406760/… 指出“代码中的大多数 cmets 实际上是一种有害的代码重复形式”。还有 - 那里有一长串的 cmets。

【解决方案10】：

尺寸大约是你的屏幕尺寸（所以去找一个大的枢轴宽屏并转动它）... :-)

开个玩笑，每个函数只做一件合乎逻辑的事情。

积极的一面是，单元测试确实更容易使用只做一件事的小逻辑函数来完成。做很多事情的大函数更难验证！

/约翰

【讨论】：

【解决方案11】：

一件事（从函数名称中应该很明显），但无论如何都不过是一屏代码。并随意增加您的字体大小。如果有疑问，请将其重构为两个或多个函数。

【讨论】：

【解决方案12】：

经验法则：如果一个函数包含执行某些操作的代码块，这与其余代码有些分离，请将其放在单独的函数中。示例：

function build_address_list_for_zip($zip) {

    $query = "SELECT * FROM ADDRESS WHERE zip = $zip";
    $results = perform_query($query);
    $addresses = array();
    while ($address = fetch_query_result($results)) {
        $addresses[] = $address;
    }

    // now create a nice looking list of
    // addresses for the user
    return $html_content;
}

好多了：

function fetch_addresses_for_zip($zip) {
    $query = "SELECT * FROM ADDRESS WHERE zip = $zip";
    $results = perform_query($query);
    $addresses = array();
    while ($address = fetch_query_result($results)) {
        $addresses[] = $address;
    }
    return $addresses;
}

function build_address_list_for_zip($zip) {

    $addresses = fetch_addresses_for_zip($zip);

    // now create a nice looking list of
    // addresses for the user
    return $html_content;
}

这种方法有两个优点：

当您需要获取某个邮政编码的地址时，您可以使用现成的功能。
当您需要再次读取函数build_address_list_for_zip() 时，您知道第一个代码块将要做什么（它获取某个邮政编码的地址，至少您可以从函数名称中得出）。如果您将查询代码保留为内联，则首先需要分析该代码。

[另一方面（我会否认我告诉过你这一点，即使受到折磨）：如果你阅读了很多关于 PHP 优化的内容，你可能会想到尽可能少地使用函数，因为函数调用在 PHP 中非常非常昂贵。我不知道这一点，因为我从未做过任何基准测试。如果是这种情况，如果您的应用程序非常“性能敏感”，您可能最好不要遵循您的问题的任何答案;-)]

【讨论】：

【解决方案13】：

以下是可能表明函数太长的危险信号列表（无特定顺序）：

深度嵌套的控制结构：例如使用具有复杂条件的嵌套 if 语句，for 循环 3 级深，甚至只有 2 级深。
太多状态定义参数：我说的状态定义参数，是指保证特定执行的函数参数通过函数的路径。获取太多此类参数，执行路径的组合就会爆炸式增长（这通常与 #1 同时发生）。
在其他方法中重复的逻辑：糟糕的代码重用是单一程序代码的巨大贡献者。很多这样的逻辑重复可能非常微妙，但一旦重构，最终结果可能是一个更加优雅的设计。
过度的类间耦合：缺乏适当的封装会导致函数涉及其他类的密切特征，从而延长它们。
不必要的开销：指出明显的、深度嵌套的类、私有嵌套类变量的多余 getter 和 setter 以及异常长的函数/变量名称的注释都会在内部产生语法噪音最终会增加其长度的相关函数。
您的大型开发级显示器不够大，无法显示它：实际上，今天的显示器已经足够大，以至于任何接近其高度的功能可能也是如此长。但是，如果它更大，这就是有问题的确凿证据。
您无法立即确定函数的用途：此外，一旦您真正确实确定了它的用途，如果您不能将这个用途概括为单句或者碰巧头疼的厉害，这应该是个线索。

总之，单一功能可能会产生深远的影响，并且通常是主要设计缺陷的症状。每当我遇到读起来绝对快乐的代码时，它的优雅就立即显现出来。猜猜看：这些函数的长度通常非常短。

【讨论】：

好帖子！恕我直言，正确的尺寸低于 80x25，带有 8 个空格标签。另一个标准（8）：代码中有重复的patterns。这可能会减少到 (3)。
@PedroMorteRolo 没错。标准 API 并不始终是优雅的典范。此外，大部分 Java API 都是在熟悉 Java 编译器和 JVM 的情况下开发的，因此您有可能解释它的性能注意事项。我承认不能浪费一毫秒的代码的关键部分可能不得不打破其中的一些规则，但这应该始终被视为一种特殊情况。预先花费额外的开发时间是一项初始投资，可以避免未来（可能严重的）技术债务。
顺便说一句.. 我认为长方法是坏的启发式也适用于类。恕我直言，长班不好，因为它们往往违反单一责任原则。让编译器为长类和方法发出警告会很有趣......
@PedroMorteRolo 我绝对同意这一点。此外，大型类可能具有更多的可变状态：这会导致代码非常难以维护。
最佳答案。另一个很好的线索是：代码中的 cmets 是什么样的？我偶然发现某人的代码的次数如下：// fetch Foo's credentials where Bar is "uncomplete"。这几乎肯定是一个函数名，应该解耦。可能想要重构为：Foo.fetchCredentialWhereBarUncomplete()

【解决方案14】：

我个人的启发是，如果不滚动就看不到整个内容，那就太长了。

【讨论】：

【解决方案15】：

我同意一个函数应该只做一件事，但那是什么级别的一件事。

如果您的 60 行正在完成一件事（从您的程序的角度来看），而构成这 60 行的部分不会被其他任何东西使用，那么 60 行就可以了。

分解它并没有真正的好处，除非你能把它分解成独立的混凝土块。使用的指标是功能而不是代码行数。

我参与过许多程序，其中作者将唯一的一件事发挥到了极致，最终所做的只是让它看起来像有人拿着手榴弹到一个函数/方法上，然后把它炸成几十个未连接的难以理解的作品。

在提取该功能的各个部分时，您还需要考虑是否会增加任何不必要的开销并避免传递大量数据。

我认为关键是要在那个长函数中寻找可重用性并将这些部分拉出来。剩下的就是函数，不管是 10 行、20 行还是 60 行。

【讨论】：

另一个重要的指标是块嵌套的层数。保持在最低限度。将功能分解成更小的部分通常会有所帮助。其他方面也有帮助，例如多次退货。
可读性是一大优势。

【解决方案16】：

请记住，您最终可能只是为了重构而进行重构，这可能会使代码比最初更难以阅读。

我的一位前同事有一条奇怪的规则，即函数/方法只能包含 4 行代码！他试图如此死板地坚持这一点，以至于他的方法名称经常变得重复且毫无意义，而且调用变得非常嵌套和混乱。

所以我自己的口头禅变成了：如果你想不出一个合适的函数/方法名称来为你重构的代码块，不要打扰。

【讨论】：

【解决方案17】：

我认为这个页面上的“只做一件事”的口头禅有一个很大的警告。有时做一件事会影响很多变量。如果较小的函数最终具有较长的参数列表，请不要将较长的函数分解为一堆较小的函数。这样做只会将单个函数变成一组高度耦合的函数，没有真正的个体价值。

【讨论】：

【解决方案18】：

已经对这个主题进行了一些深入的研究，如果你想要最少的错误，你的代码不应该太长。但它也不应该太短。

我不同意一个方法应该适合您的显示器，但是如果您向下滚动超过一页，则该方法太长了。

看 The Optimal Class Size for Object-Oriented Software 进一步讨论。

【讨论】：

【解决方案19】：

我之前写过 500 行函数，但是这些只是用于解码和响应消息的大开关语句。当单个消息的代码比单个 if-then-else 更复杂时，我将其提取出来。

本质上，虽然函数是 500 行，但独立维护的区域平均为 5 行。

【讨论】：

【解决方案20】：

看一下 McCabe 的圈，他在其中将代码分解成一个图，其中“图中的每个节点对应于程序中的一个代码块，其中流程是顺序的，弧对应于引入的分支程序。”

现在假设您的代码没有函数/方法；它只是图表形式的一大堆代码。

您想将这种蔓延分解为方法。考虑一下，当你这样做时，每个方法中都会有一定数量的块。每个方法中只有一个块对所有其他方法可见：第一个块（我们假设您只能在一个点跳转到一个方法：第一个块）。每个方法中的所有其他块都将是隐藏在该方法中的信息，但方法中的每个块可能会跳转到该方法中的任何其他块。

要根据每个方法的块数确定方法的大小，您可能会问自己一个问题：我应该使用多少方法来最小化所有块之间的最大潜在依赖数 (MPE)？

这个答案是由一个方程式给出的。如果 r 是最小化系统 MPE 的方法数，n 是系统中的块数，则等式为： r = sqrt(n)

可以证明，这给出了每个方法的块数，也是 sqrt(n)。

【讨论】：

【解决方案21】：

我通常使用测试驱动的方法来编写代码。在这种方法中，函数大小通常与测试的粒度有关。

如果您的测试足够集中，那么它将引导您编写一个小的集中函数以使测试通过。

这也适用于另一个方向。函数需要足够小才能有效地测试。因此，在处理遗留代码时，我经常发现我分解了更大的函数以测试它们的不同部分。

我通常会问自己“这个功能的职责是什么”，如果我不能用一个清晰简洁的句子来说明职责，然后将其翻译成一个小的重点测试，我怀疑这个功能是否太大了。

【讨论】：

【解决方案22】：

延续 Bob 叔叔的推文精神，当您觉得需要在两行代码之间放置一个空行时，您就知道函数变得太长了。这个想法是，如果你需要一个空行来分隔代码，那么它的职责和范围就是分开的。

【讨论】：

【解决方案23】：

如果分支多于三个，一般是把一个函数或方法拆开，将分支逻辑封装在不同的方法中。

每个 for 循环、if 语句等都不会被视为调用方法中的一个分支。

用于 Java 代码的 Cobertura（我确信还有其他用于其他语言的工具）为每个函数计算函数中 if 等的数量，并将其求和为“平均圈复杂度”。

如果一个函数/方法只有三个分支，它会在那个指标上得到三个，这非常好。

有时很难遵循此准则，即验证用户输入。尽管如此，将分支放在不同的方法中不仅有助于开发和维护，而且还有助于测试，因为可以轻松分析执行分支的方法的输入，以查看需要将哪些输入添加到测试用例中以覆盖分支没有被覆盖。

如果所有分支都在一个方法中，则必须从方法开始就跟踪输入，这会妨碍可测试性。

【讨论】：

【解决方案24】：

我的想法是，如果我不得不问自己是否太长，那可能是太长了。它有助于在这个领域制作更小的函数，因为它可以在应用程序的生命周期后期提供帮助。

【讨论】：