#1 楼
如果无法审查，则无法通过审查。您必须了解，代码审查不是为了查找错误。这就是质量检查的目的。代码审查旨在确保将来可以维护代码。如果您现在甚至无法遵循该代码，那么如何在六个月内被指派进行功能增强和/或错误修复？现在发现错误只是一个附带好处。
如果太复杂了，那就违反了很多SOLID原则。重构，重构，重构。将其分解为正确命名的函数，它们的功能要简单得多。您可以清理它，并且您的测试用例将确保它继续正确运行。您有测试用例，对吗？如果不是，则应开始添加它们。
评论
这个非常。请记住，不仅您和将要阅读此代码的作者。这也将是10年内的一些随机实习生，因此您要确保他有机会了解正在发生的事情。
–大卫说恢复莫妮卡
16 Dec 15 '21：30
好答案。这取决于“代码审查”的目的。可读性是一回事，结构又是另一回事-但它们之间息息相关。之前，我正在与MAJOR军团编写的一些开放源代码一起使用，由于var和fn的名称很容易理解，因此几乎不可读。
–user223083
16年12月15日在22:41
@DavidGrinberg实际上，“六个月内的你”是一个完全不同的人。
–克莱里斯-谨慎乐观-
16 Dec 15 '23：40
将代码放置一段时间（足够长的时间让他不记得所有内容）。请原始编码器对其进行审查。看看他是否理解。
–尼尔森
16/12/16在4:21
我不同意代码审查“不是”以发现错误。它经常会发现错误，这是代码审查的一个非常强大和有用的方面。更好的是，它有助于找到在将来的代码中完全避免错误的方法。重点可能是高估了，应该不是仅仅为了发现错误！
–科迪·格雷
16 Dec 16 '15：32
#2 楼
您提到的所有内容都可以在代码审查中明确指出。当我收到代码审查时，我也会对测试进行审查。如果测试没有提供足够的覆盖率，那是需要指出的。这些测试必须对确保代码按预期工作并将在更改中继续按预期工作有用。实际上，这是我在代码审查中寻找的第一件事。如果您还没有证明自己的代码符合要求，那么我就不想花时间在代码上。或难以遵循，这也是人类应该注意的事情。静态分析工具可以指出一些复杂性的度量标准，并标记过于复杂的方法，以及发现代码中的潜在缺陷（并应在人工代码审查之前运行）。但是代码是由人类读取和维护的，因此需要首先编写代码以确保可维护性。仅当有理由使用较少可维护的代码时，才应以这种方式编写。如果您需要复杂或不直观的代码，则应记录下来（最好在代码中）为什么使用这种代码，并提供有用的注释，以供将来的开发人员了解代码的用途和作用。
理想情况下，拒绝没有适当测试或代码过于复杂而没有充分理由的代码审查。前进可能有商业原因，因此您需要评估风险。如果您确实要承担代码中的技术债务，请立即将票证放入Bug跟踪系统，其中包含需要更改的内容的详细信息以及更改建议。
#3 楼
但是，即使有可能，通过代码审查手动验证整体正确性也是非常困难且耗时的。评论。进行代码审查的方式是想象代码中存在错误，而您必须对其进行修复。以这种思维方式，浏览代码（尤其是注释）并问自己：“是否容易理解正在发生的事情，以便我缩小问题范围？”如果是这样，那就是通行证。否则，将会失败。至少至少需要更多的文档，或者可能需要进行重构才能使代码合理地理解。大多数代码是如此糟糕，以至于可以轻松地将其连续重构10次，每次都变得更具可读性。但是您的雇主可能不想为拥有全球最易读的代码而付费。
评论
很棒的评论！ “大多数代码是如此糟糕，以至于可以轻松地将其连续十次进行重构，每次都变得更具可读性。”
–迪恩·拉德克利夫（Dean Radcliffe）
16 Dec 16'在5:30
“大多数代码是如此糟糕，以至于可以轻松地将其连续十次进行重构，并且每次都变得更具可读性。”确实，这就是现实世界中的情况。
– Peter Mortensen
16 Dec 17 '21：49
@PeterMortensen确实，您在现实世界中发现了很多东西。但是以这种方式编写代码并不符合任何人的利益。我认为这样做的原因有两个。开发人员接受的教育很少投入精力来教如何编写可读代码。在某些企业中，这被认为是浪费时间：“如果开发人员已经编写了工作代码，我们为什么要关心它是否可读？只管把它寄出去。”
–卡巴斯德
16 Dec 18'在12:56
#4 楼
但是，即使有可能，通过代码审查手动验证整体正确性也是非常困难且耗时的。实际上，我的工作就是通过对学生的作业进行评分来做到这一点。虽然许多人提供了一些合理的质量，但到处都有错误，但仍有两个人脱颖而出。两者都始终提交没有错误的代码。我提交了一个可以从顶部和底部高速读取的代码，并用零的努力将其标记为100％正确。另一个提交的代码是另一个WTF，但是以某种方式设法避免了任何错误。要标记的绝对痛苦。
今天，第二个代码将在代码审查中被拒绝。如果验证正确性非常困难且耗时，那么这就是代码的问题。一位体面的程序员会弄清楚如何解决问题（花费时间X），并且在进行代码审查之前将其重构，这样它不仅可以完成任务，而且显然可以完成任务。它所花费的时间比X少得多，并在将来节省了大量时间。通常是在漏洞进入代码审查阶段之前就发现它们。接下来，使代码审查快得多。并在未来的任何时候都使代码更容易适应。
另一个回答是，某些人的代码可以重构10次，每次都变得更具可读性。真是可悲。那是一个应该寻找其他工作的开发人员。
评论
我将代码重构10次所需的时间要少得多，然后我需要编写代码的第一个版本。如果其他人知道我已经完成了此重构，那么我将失败。
–伊恩
16 Dec 19 '14：45
#5 楼
这个旧代码是否做了些改动？ （在10000行代码库中更改了100行代码仍然是一个很小的更改）有时存在时间限制，开发人员被迫留在一个旧的不方便的框架中，仅仅是因为完全重写将花费更长的时间并且超出预算。 +通常会涉及风险，如果评估不当，可能会损失数百万美元。如果它是旧代码，则在大多数情况下，您将不得不忍受它。如果您自己不了解它，请与他们交谈，并听听他们的意见，然后尝试理解。请记住，对您而言可能很难遵循，但对其他人而言却完全可以。站在他们的一边，从他们的末端看。这是新代码吗？根据时间限制，您应该提倡尽可能多地重构。如有必要，可以花更多时间进行代码审查吗？您不应该将自己的时间限制在15分钟以内，了解一下并继续前进。如果作者花了一周时间写东西，可以花4到8个小时来复习。您在这里的目标是帮助他们重构。您不只是返回说“立即重构”的代码。看看可以分解哪些方法，尝试提出一些想法来引入新的类等。
评论
您不只是返回说“立即重构”的代码-为什么？我至少收到过一次这样的评论，而上次我记得它证明是有用且正确的。我必须从头开始重写大量代码，这是正确的做法，因为回首过去，我自己看到旧代码是无法挽救的烂摊子。审阅者足够有资格注意到这一点（而我显然不是）
– gna
16 Dec 16'在21:20
@gnat：一方面，因为它很粗鲁。当您解释代码出了什么问题并努力帮助他人进行改进时，您会看起来更好。在大型公司中，否则可能会迅速将您带出门。特别是如果您查看高级人员的密码。
–Neolisk
16 Dec 16 '21：24
我在上面提到的那种情况，是在一家大型公司中，刚好足够小心不要离开他们最有资格的开发人员，至少不是以被要求时直接分享他们的技术专长为由。
– gna
16 Dec 16'在21:29
@gnat：“立即重构”的方法可能会向下起作用，即，具有10年以上经验的高级开发人员说要重构为一个月前没有经验或类似情况的新开发人员。向上-您可能有问题。因为您可能不知道其他开发人员有多少经验，所以可以将尊重视为默认行为。它肯定不会伤害您。
–Neolisk
16 Dec 16'在21:39
@Neolisk：一个经验丰富的开发人员不得不在时间压力下编写代码，并且知道它不够好，可能只有在您拒绝该代码的情况下才会感到高兴，这给了他时间和改善它的借口。 PHB认为它足够好会使开发人员感到不满意。评论者认为不够好会使他感到高兴。
– gnasher729
16 Dec 18'在14:09
#6 楼
通常，“复杂的”补丁/变更列表是同时执行许多不同功能的补丁/变更列表。有新代码，已删除代码，重构代码，移动代码，扩展测试；一个常见的线索是，补丁很大，但描述很小：“实现$ FOO。”
处理此类补丁的方法是要求将其分解为一系列较小的独立组件。就像单一职责原则说的那样，一个功能应该只做一件事，一个补丁也应该只关注一件事。例如，第一个补丁可能包含纯机械重构，而没有任何功能更改，然后最终补丁可以将精力集中在$ FOO的实际实现和测试上，从而减少干扰和红色鲱鱼。
对于需要大量新代码的功能，新代码通常可以被引入可测试的块中，这些块不会改变产品的行为，直到该系列的最后一个补丁实际调用新代码（标志翻转）为止。
它是我的问题，然后寻求作者的帮助：“我在跟踪这里发生的所有事情时遇到了麻烦。您能否将此补丁分解为更小的步骤，以帮助我了解所有这些如何结合在一起？”有时有必要针对较小的步骤提出具体建议。
如此大的补丁（例如“ Implement $ FOO”）会变成一系列补丁，例如：
一个新版本的Frobnicate，它需要一对迭代器，因为我需要用vector以外的序列来调用它来实现$ FOO。
将所有现有的Frobnicate调用者切换为使用新版本。 />删除旧的Frobnicate。
Frobnicate做得太多。将折回步骤纳入其自己的方法中，并为此添加测试。
将Zerzify引入测试。尚未使用，但我需要$ FOO。
根据Zerzify和新的Frobnicate实现$ FOO。
请注意，步骤1-5不会对产品进行功能更改。他们很容易审查，包括确保您拥有所有正确的测试。即使第6步仍然很“复杂”，至少它也将重点放在$ FOO上。而且日志自然可以让您更好地了解如何实现$ FOO（以及为何更改Frobnicate）。
评论
如果使用Git，一种方法是编写多个提交的请求请求。每个提交都尽可能地原子和独立，并具有自己的描述。然后，在PR正文中添加一个有用的注释，即可以手动检查每个更改。通常，这就是我处理大型PR的方式，例如全局重构或无懈可击的大型工具变更。
–吉米·布雷克·麦基（Jimmy Breck-McKye）
16/12/19在10:35
#7 楼
正如其他人指出的那样，代码审查并非真正旨在发现错误。如果您在代码审查期间发现错误，则可能意味着您没有足够的自动化测试范围（例如，单元/集成测试）。如果没有足够的覆盖范围使我相信代码可以完成预期的工作，那么我通常会要求进行更多测试，并指出所需的测试用例，并且通常不允许代码进入覆盖范围不足的代码库。如果高层体系结构太复杂或没有意义，我通常会召集几个团队成员开会讨论这个问题。有时很难在糟糕的体系结构上进行迭代。如果开发人员是新手，我通常会确保我们提前了解他们的想法，而不是对糟糕的请求提出反应。如果问题没有明显的解决方案（很可能会被选中），即使对于经验丰富的开发人员也通常如此。
如果复杂度被隔离到通常可以迭代地进行校正并通过良好的自动化测试的方法级别。
最后一点。作为审阅者，您需要确定代码的复杂性是由于本质的还是偶然的复杂性。本质上的复杂性与软件中确实难以解决的部分有关。偶然的复杂性指的是我们编写的代码的所有其他部分，它们无缘无故地过于复杂，可以轻松地简化。
我通常会确保具有本质复杂性的代码确实如此，并且不能进一步简化。我还旨在为这些零件提供更多的测试范围和良好的文档资料。在请求请求过程中，几乎总是应该清除意外的复杂性，因为这些是我们处理的大部分代码，即使在短期内也很容易引起维护的噩梦。
评论
出现还是？快速评估源文件将确认或否认您的怀疑。测量后，只需在代码审查中调出测量编号，并建议进行重构以降低复杂度编号。相关：在审核过程中，如何在提示上建议他人改进设计不良的代码？
可能重复的代码复习太难了怎么办？
请定义“太复杂”。代码是否过于复杂，是因为它使用了您的团队不熟悉的众所周知的设计模式，还是因为它没有使用您的团队所熟知的模式？判断代码“太复杂”的确切原因对于创建正确的评估方法至关重要。在诸如代码审查之类的深而复杂的知识领域中，诸如“太复杂”之类的简单陈述暗示着开发者对我的猎杀。
@PieterGeerkens还是因为解决一个复杂的问题而太复杂了？