你能容忍多少重复的代码?

54

在最近的代 码审查中,我发现一个类中有几行重复逻辑(不到15行)。当我建议作者重构代码时,他辩称那样写的代码更容易理解。但是重新阅读代码后,我必须承认提取重复逻辑可能会影响可读性。

我知道DRY原则只是指导方针,而不是绝对规则。但一般情况下,你是否愿意在追求DRY原则的名义下损害可读性?


8
这样的投票应该设为社区维基。 - anon
8
我愿意修复同一个错误的次数是多少?换句话说,零次。 - No Refunds No Returns
15
可以提供一些代码吗?我很难想出一段看起来比非重复的代码更好的重复代码。 - Samuel Carrijo
1
如果 (str == null || str.length() == 0) vs isStringNullOrEmpty(str),哪一个更好呢?我知道哪一个更容易一遍又一遍地输入。我知道哪一个在添加另一个测试时更具描述性和可维护性。如果一行代码可以重构为一个更具描述性的函数,我无法想象在25年的编程中会有15行代码不能被命名为一个函数。 - user177800
1
@fuzzy:当你“想要添加另一个测试”时,isStringNullOrEmpty的描述性较差。那段代码总体上只是一个糟糕的例子。 - Roger Pate
显示剩余5条评论
13个回答

72

重构:改善现有代码的设计

三次律

第一次做某件事,你只是去做。第二次做类似的事,你会犯错并且觉得重复,但还是会继续重复。第三次做类似的事情,你就需要进行重构。

三次失败就进行重构。


编码人生

Seibel: 所以对于这些 XII 调用,你写了一个实现。
你有没有发现你积累了许多非常相似的代码片段?

Zawinski: 哦,是的,肯定的。通常到第二或第三次复制和粘贴那段代码时,就需要停下来把它放在子程序中。


8
+1 - 三倍法则不仅仅是一种经验法则,它已经被研究过。它适用于小的冗余部分以及整个框架(如产品线驱动开发)。与复制/修改相比,实现重用所需的回报率约为2.5。 - Chris Kessel
5
我认为应该是“两个人的规则”。请查看我对这位提问者问题的回答。 - Ira Baxter
2
第一次不是重复。第二次才算是重复。第三次将会是第二次重复,这是我们应该进行重构的提示。关键在于,并不总是要尝试将某些东西拆分出来使其可重用,当你只使用过它一次时。 - Steven Sudit
1
因此,需要适度抽象和跟踪代码克隆的位置。如果你能做到这一点,就不需要那么多抽象了。如果做不到这一点,最好在第二个上进行抽象,因为当第一个在两年后需要更新时,你永远找不到第二个克隆体。 - Ira Baxter
就像在《异域镇魂曲》中一样,三倍规则同样适用。第一次通常没有时间去做那件事。第二次,通常没有足够的时间来调试一个问题三次。这时我会进行重构。 - gruszczy
显示剩余2条评论

42

个人而言,我更喜欢首先让代码易于理解。

DRY的目的在于简化代码的维护。为了消除重复的代码而使代码变得难以理解,在许多情况下会损害代码的可维护性,而不如有一些重复的代码行。

话虽如此,我确实认为DRY是一个好的目标,如果实际可行的话。


8
我同意。DRY是用来实现可维护性和可读性的工具,而不是目标本身。请注意,原文中的DRY是一个缩写,表示"Don't Repeat Yourself"(不要重复自己)。 - Ray
7
不要重复自己。 - OregonGhost
1
@Reed:没错。我也找不到这样的东西。如果Sly能像Samuel在问题评论中所说的那样提供一些代码,这个讨论会更有启发性。 - Vinko Vrsalovic
3
@Vinko:我只在代码总行数非常短(总共2-3行)并且必须在重复的代码之间运行逻辑时才看到过这种情况。这可以使用函数式风格进行重构,以实现DRY原则,但这往往会增加维护成本,因为它几乎总是需要引入方法指针/lambda/delegate等(具体取决于编程语言)。 - Reed Copsey
1
如果是像 if(test == -1 || test > 5) 这样的小段代码,我会直接重复使用。但如果是查询数据库并从不同表中检索一组数据这样的大段代码,我肯定会制作一个子程序。重构一个充满重复的大型应用程序的成本很高:在这种情况下,我不会这么做。如果你早期修复了问题,那就很好。否则,算了吧。只需保留重复的部分。 :P - Jimmie Lin
显示剩余11条评论

42

我对重复代码毫不容忍。虽然有时因为时间限制等原因可能会出现重复代码,但我仍未找到真正需要重复代码的情况。

称重复代码会影响可读性只是表明你在选择命名方面不够好 :-)


11
对于关于挑选好名称的评论点赞。对我来说,很难想象使用一个合理的名称来调用方法会比写十几行代码更加易读......特别是当你看到同一个方法第二次被调用时,你不必在阅读一堆代码后才能确定“是的,这和以前一样”。 - Tim Goodman
7
我也不容忍重复的代码。重复的代码会在未来给你带来麻烦。 - GmonC
+1 是“选择不好的名称”。 另外,可能是因为您进行了不幸的重构。尝试重构一个单独有意义的部分(尽可能地),然后重构实际上将通过将实现细节隐藏在漂亮的方法名称背后来提高可读性。 - sleske
2
我不想容忍任何问题,但实际的预算限制会决定我在项目上花费多少时间来进行重构。另一种选择是免费重构代码。 - Peter M
@Peter M:没错。我的许多项目我都无法容忍。它们能够工作并且已经投入生产并不能改变这一事实。我很想改进它们。 - Vinko Vrsalovic
4
也许之所以难以找到名称是因为它没有对应一个好的、可封装的概念。也许重复的代码并不是一个合理的独立功能单元。如果在阅读代码前必须追踪代码,则会影响可读性,而且你必须阅读它,因为你将代码分成了不合理的块。正如sleske所说,根据有意义的内容进行重构,而不仅仅是根据重复的部分进行重构。 - Darryl

12

如果所涉及的代码有明确的业务或技术支持目的P,通常应该进行重构。否则,你将面临克隆代码的典型问题:最终你会发现需要修改与P支持相关的代码,但你找不到所有实现它的克隆。

有些人建议将克隆代码数量阈值设为3个或更多个时进行重构。我认为,如果有两个克隆代码,也应该进行重构;在大型系统中,找到其他克隆代码(甚至知道它们可能存在)是很困难的,无论你有两个、三个还是更多。

现在,这个答案是在没有任何能够“找到”克隆代码的工具的情况下提供的。如果你可以可靠地找到克隆代码,那么进行重构的原始理由(避免维护错误)就不那么有说服力了(具有命名抽象的效用仍然是真实的)。你真正需要的是一种可以找到和跟踪克隆代码的方法;抽象它们是一种确保你可以“找到”它们的方法(通过使查找变得微不足道)。

一种能够可靠地找到克隆代码的工具至少可以防止你犯“未更新克隆代码维护错误”的错误。其中一种工具(我是作者)是CloneDR。CloneDR使用目标语言结构作为指导来查找克隆代码,因此无论空白布局、注释更改、变量重命名等都能找到克隆代码(它适用于许多语言,包括C、C++、Java、C#、COBOL和PHP)。CloneDR将在大型系统中查找克隆代码,而不需要任何指导。检测到的克隆代码将被显示出来,以及 antiunifier,这本质上是你可能写的抽象化版本。它的某些版本(针对COBOL)现在已经与Eclipse集成,并在缓冲区内显示你是否正在编辑克隆代码,以及其他克隆代码的位置,以便你可以在那里检查/修订其他克隆代码(你可能会进行重构:)。

我曾经认为克隆就是彻头彻尾的错误,但人们会这样做是因为他们不知道克隆体与原始体的差异,所以在克隆行为发生时最终抽象化并不清晰。现在我相信,如果您可以跟踪克隆体,并在抽象化变得清晰后尝试进行重构,那么克隆是有益的。


3
我同意你的观点。如果重复的代码反复执行相同的操作,那么它所执行的内容应该有一个名称,并且值得被提取出来。这样可以根据该名称的语义而不是语法细节来查看代码,简化维护工作,实际上使代码更易于阅读。 - Steven Sudit
2
+1:实际上,许多敏捷方法都推荐这种方法:首先实现新功能,根据需要进行复制和粘贴。然后,在完成后进行重构。其想法是在实现完成后,您将更好地了解哪种抽象适合。 - sleske
1
你不能构建抽象,直到你理解应该被抽象化的内容。如果你能够看着一段代码并神奇地知道如何产生抽象,那么你可以采用纯粹主义政策。但由于你不能这样做,你必须实际上分阶段进行此过程...其中第一步是实际克隆。现在,当你完成时(可能不仅仅是修改的那一部分),你应该尝试抽象化。但是...你的语言可能没有合适的抽象机制(COBOL没有泛型)...所以如果你无法去克隆,你应该跟踪。 - Ira Baxter
当然,你面临的真正问题是要维护一个有克隆系统的遗留系统(这是确定的,我敢打赌100K SLOC以上的任何东西都至少有10%的克隆),所以...你正在编辑的代码块是否是某个东西的克隆?你需要工具来查找和跟踪那些你还没有机会找到并消除的克隆。 - Ira Baxter
1
当语言不允许适当的抽象时,完全同意。但是当它们允许时,您绝对可以在不完全理解(可能错误地)的情况下进行抽象,然后在看到它失败时重构抽象。如果您意识到这是一个错误,甚至可能最终会具体化这个错误的抽象。 - Vinko Vrsalovic
显示剩余6条评论

9
只要你重复任何内容,就会创建多个位置进行编辑,如果发现错误,需要扩展、编辑、删除或其他可能导致必须更改的原因,这些原因有几十种。

在大多数语言中,将块提取到适当命名的方法中很少会影响可读性。

这是您自己的代码,符合您的标准,但我对您的“多少”的基本答案是没有


3

你没有说明是哪种语言,但在大多数IDE中,这是一个简单的“重构”->“提取方法”。这样做有多容易,一个带有一些参数的单一方法比两个重复代码块更易于维护。


1
@fuzzy:我的观点不是提取代码很难;这只需要几秒钟的时间。我的观点是,这会损害可读性,因为它会提高抽象级别。 - Sylvain
2
@Sly:我很难相信你找不到一个能使函数的命名至少和原始代码一样易读的名称。 - Vinko Vrsalovic
2
它可能会影响抽象化代码的可读性,但很可能不会;明确知道变化点是好的。但是,在使用时调用站点现在将说“DoFrobbish(...)”,其中Frobbish为系统维护者所熟悉。这比“这里有15行代码,你猜它是做什么”的理解要容易得多。(PS:是15行吗?17行?12行?你怎么知道?)调用良好命名的抽象化对于读者来说是一个大胜利。 - Ira Baxter
@Ira Baxter: 除了随机的十几行代码,它并不能构成一个可命名的函数。如果你可以指出重复之处并描述其含义,那是可以的,但基于代码结构来拆分程序是不好的做法。 - David Thornley
2
@David:如果代码不可描述或者无法“函数化”,为什么你要重复它呢? - Vinko Vrsalovic
1
@David:是的,这段代码块必须具有可识别的目的。请查看我对这个问题的回答。如果有的话,您应该能够给这个目的起一个好的(也许是特定于领域的)名称。 - Ira Baxter

3

抽象地说很难确定。但我自己的信仰是,即使只有一行重复的代码,也应该被制作成一个函数。当然,我自己并不总能达到这个高标准。


如果那一行代码有特定的功能,我同意。 - Steven Sudit
3
您需要设置较低的阈值。如果将ab参数化,则它是xy的克隆,但它不是一个有趣的克隆。我已经构建了约10年的克隆检测器(http://www.semanticdesigns.com/Products/Clone),我的经验表明,“几行”是检测它们的良好阈值。然而,如果您有一个“一行克隆”,它具有足够的结构和足够的业务或技术相关性,那么您应该进行重构。我的克隆检测器通过测量“克隆质量”而不是“行数”来接近这个理想。 - Ira Baxter
5
@Tim: “这样有什么好处?” 之所以更好,是因为它给那个语句/表达命名了。实际上,因为这个原因,即使那个语句没有在别处重复,我也经常提取单个语句的方法。例如,我经常在LINQ表达式中这样做,仅仅为了可读性。 - Sylvain
1
@Tim:假设你要换行的那一行代码比较复杂,包含了一些不太明显的逻辑,最好用一个解释性的名称来隐藏它。例如,如果它在表格中查找一个值并且在该值在某个范围内时返回true,那么最好使用IsLegalSize()或者其他最适合领域的名称。 - Steven Sudit
1
@Sly, @Steven:说的有道理,如果那一行代码很难理解,那么一个单行方法肯定能提高可读性。当然,在那种情况下,重复并不是真正的问题(尽管我想,让一行难以理解的代码出现两次要比出现一次更糟糕……) - Tim Goodman
显示剩余2条评论

3
重构可能很困难,这取决于编程语言。所有语言都有局限性,有时重构后的代码逻辑比重复的代码还要语言上更加复杂。
通常,当具有不同基类的两个对象在操作方式上存在相似之处时,代码逻辑的重复出现。例如,两个 GUI 组件都显示值,但没有实现用于访问该值的公共接口。重构这种系统需要使用比所需更通用的方法,然后进行类型检查和强制转换,或者需要重新思考和重构类层次结构。
这种情况与代码完全重复不同。如果我只打算将其用于两次,并且两次都在同一个函数中,则不一定会创建新的接口类。

1
DRY 的目的是为了可维护性。如果代码难以理解,那么它就更难以维护,因此,如果重构会影响可读性,你实际上可能没有达到 DRY 的目标。对于少于 15 行代码,我倾向于同意你的同学的观点。

2
为什么会影响可读性? - Steven Sudit
2
首先,因为原始问题说过了。他和他的同学都同意。但除了仅仅相信提问者所说之外,让我们看看为什么会这样。如果是完全出于代码重复而进行重构的驱动力,那么重复的代码块可能并不对应一个良好的抽象概念。如果你必须定位和检查代码以了解它如何影响调用函数,那么你增加了复杂性而不是减少它。仅有重复并不足以理由来提取一个代码块。需要同时具备重复和一个良好的概念才可以。 - Darryl
@StevenSudit 考虑将 if/else 逻辑流程抽象成一个通用方法,该方法以一些函数作为其参数(“如果成功则执行 f(x),否则执行 g(x) 等)。我认为这种方法不如在方法之间重复使用惯用的 if/else 流程易读。我还会说,这种抽象增加了严格的限制,可能会导致后续的可维护性问题(如果一个客户端更改了其 if/else 流程要求)。 - T_T

1
一般来说,不会。至少对于可读性而言是这样的。总有一些方法可以将重复的代码重构成一个意图清晰的通用方法,就像读书一样,我个人认为。
如果你想为了避免引入依赖而违反DRY提出论点,那可能更有说服力,并且你可以在这里获得Ayende的主观意见以及代码来说明这一点。
除非你的开发人员真的是Ayende,否则我建议坚持DRY原则,并通过意图清晰的方法来提高可读性。
BH

网页内容由stack overflow 提供, 点击上面的
可以查看英文原文,
原文链接