五种代码评审误区(code review) (QFY译版)

by Trisha Gee
May 4, 2020
原文 : https://blogs.oracle.com/javamagazine/five-code-review-antipatterns

每个人都关心最佳实践,但错误的实践有时会更具启发。
代码检查是必要的,但不一定都是正确的方式。本文指出了开发人员在审查代码或提交合并请求(Pull request) 时可能遇到的一些错误方式。

误区:挑剔

想象一下下面的场景。代码作者花了数小时甚至数天来创建他们认为最合适的解决方案。他们考虑了多种设计方案,采用了似乎最合适的方案。他们考虑了现有应用程序的结构,并在适当的地方进行了更改。然后他们将解决方案作为一个pull请求提交,或者他们开始了代码评审,他们从专家收到的反馈是:

  • “您应该使用制表符,而不是空格”

  • “我不喜欢括号在这一块.”

  • “文件末尾没有空行.”

  • “您的枚举是大写的,应该是采用大小写方式.”

尽管新代码与现有代码风格保持一致很重要,但这些都不需要人工审查。人工审查很昂贵,能完成计算机不能完成的事情。是否符合编码风格计算机很容易检查,人工做会分散代码检查的专注点。
如果开发人员在代码评审期间看到许多此类注释,则表明团队要么没有样式指南,要么有样式指南,但检查样式未自动化。解决方案是使用诸如 checkstyle 之类的工具来确保遵循了样式指南,或者使用 sonarqube 来确定常见的质量和安全问题。持续集成可以做到这一点,而不要依赖人工审查来警告此类问题。

有时,可能很难自动执行此类检查。 例如,如果不存在代码准则,或者内部代码样式随着时间的推移而发展不同部分中有所不同。这种情况下,有一些方法可以使用自动检查。例如,团队可以一次性替换成新约定的代码风格,不包含其他更改。或者,一个团队可以约定,因bug或特性更改一个文件时,该文件也将更新为新风格,并且自动化工具可以配置为只检查更改的文件。
如果一个团队有各种各样的代码风格,并且没有自动检查样式,那么很容易陷入下一个陷阱。

误区:不一致的反馈

对于每一个被邀请来审查代码的开发人员,会出现不同意见。每个人可以同时持有多个观点。有时,代码评审会陷入评审人员之间关于不同实现方法的争论,例如流还是经典for循环是最好的。如果团队成员对同一代码看法不一致,那么开发人员该如何进行更改、关闭评审或发布到生产环境?
即使是一个评审员的想法也很容易改变,无论是在一次评审中还是在一系列评审中。在一次审查中,审查员可能会要求作者确保使用具有 O(1) 读取操作的数据结构。而在下一次审查中,审查员可能会问为什么不同地方有多种数据结构,并建议通过对单个结构进行线性搜索来简化代码。
当一个团队不清楚自己的“最佳实践”是什么样的,也不知道自己的优先级是什么时,就会出现这种情况,例如:

  • 代码是否应该朝着更现代的Java风格发展?或者代码一致更重要,因此在任何地方继续使用“经典”结构?

  • 对系统所有部分的数据结构进行 O(1) 读操作是否重要?或者有哪些地方可以接受 O(n) 吗?

几乎所有的设计问题都可以用“这取决于”来回答。为了更好地了解答案,开发人员需要了解应用程序和团队的优先级。

误区:最后一分钟更改设计

代码评审时开发人员可能得到最令人沮丧的反馈是,评审人员根本不同意解决方案的设计或体系结构,并要求完全重写代码,或者通过一系列评审(见下一节)逐步重写,或者粗暴地拒绝代码并让作者重新开始。
代码评审不是评审设计的正确时机。如果团队遵循 代码评审把关,那么代码应该可以工作,并且在让另一个开发人员评审代码前所有测试都已经通过。此时,数小时、数天甚至可能数周的成果已经进入到正在评审的代码中(虽然我真的希望不是数周;代码评审代码量应该很小,但这是另一个主题)。在代码评审期间指正底层设计错误是浪费每个人的时间。
代码评审可以用作设计评审,但是如果这是代码评审的目的,那么应该在开始实现的时候进行。然后,在开发人员编写太多代码前,他们可以用一些存根类和方法以及一些有意义的名称和步骤的测试勾勒出他们的想法,还可以提交一些文本或图表,以便让团队成员就要采取的方法给出反馈。
如果团队成员在网关评审中发现了真正的设计问题(即,当代码完成并工作时),那么团队应该更新其流程,以便更早发现这些问题。这可能意味着要进行其他类型的审查,如前一段中建议的审查、白板想法、配对编程,或者与技术主管讨论建议的解决方案。在最终的代码评审中发现设计问题是浪费开发时间,并且极大地挫伤了编码者的士气。

误区:踢皮球评审

理想情况下,作者提交代码以供审阅,评审人会发表一些带有明确解决方案的评论,作者会根据建议更改并重新提交代码,审阅将完成,代码将被推送。但如果这种情况经常发生,谁能证明代码审查过程是正确的呢?

在现实生活中,经常发生的是:

  • 开始代码审查。

  • 一些评审人提出了一些建议:一些小而简单,一些琐碎也没有明显的解决方案,还有一些复杂。

  • 作者做了一些修改:至少是简单的修改,或者是几处修改,以使评审人满意。作者可以向评审员提出问题以澄清问题,或者作者可以发表评论以解释为什么没有做出特定的更改。

  • 然后评审人,接受一些更新,对其他做进一步评论,找到其他他们不喜欢的原始代码,回答问题,并与其他评审人或作者就他们在审阅中的评论进行争论。

  • 代码作者进行了更多的更改,添加了更多的注释和问题,等等。

  • 评审人检查更改,提出更多的评论和建议,等等。

  • 第5步和第6步重复,也许永远重复。 在这过程中,理论上,在代码准备好之前,更改和注释应该减少到零。最令人沮丧的情况是每次迭代带来的新问题至少和旧问题一样多。在这种情况下,团队已经进入了“代码审查死循环”阶段,原因有很多:

  • 如果评论者吹毛求疵,并且反馈不一致,就会发生这种情况。对于养成这些习惯的评审人来说,有无穷多的问题需要识别,也有无穷多的评论需要发表。

  • 当评审没有明确的目的或评审过程中没有遵循的指导方针时,就会发生这种情况,因为每个评审员都觉得必须找到所有可能的问题。

  • 当评审人的评论没有明确说明需要代码作者提供什么时,就会发生这种情况。每条评论是否意味着必须做出改变?所有的问题是否意味着代码不是自文档的,需要改进?或者,一些评论仅仅是为了提醒代码作者下次改进,而提出的问题仅仅是为了帮助评审人理解和学习吗? 评论应该明确是否现在必须修改,如果必须要更改代码,评审人需要明确说明代码作者应该采取什么操作。
    同样重要的是要了解谁决定评审是否“完成”。可以通过项目任务清单检查来实现,也可以由某人说“评审通过”。 通常需要有人打破僵局,解决分歧。这可能是高级开发人员、领导或架构师,甚至可能是团队中高度信任的代码作者。但在某个时刻,需要有人说“评审结束了”或“当这些步骤完成时评审就结束了”

误区: 评审拖拉

这里是我承认代码评审我犯下的最大的错误:拖拉。无论我是一个评审员还是一个代码作者,在代码评审时无回应(有时在开始的时候!)。也许有一个重要的或有趣的功能,需要我审查,但我暂时不管它直到“一个更好的时间”可以“真正正确地看待它”。或者也许审查代码量比较大我想留出足够的时间。或者我是作者,在反反复复(或二十次)修改之后,我再也无法面对阅读和回答评论了,所以我决定等等“直到头脑清醒”
听起来很熟悉?
不管是什么原因,有时审查过程中的某个人根本没有回应。这可能意味着,在此人查看代码之前,该评审将处于停滞状态。这是一种浪费:虽然有人在创建资产(新代码)上投入了时间,但在投入生产之前,它并没有增值。事实上,当它越来越落后于其他代码,它可能会腐烂。(QFY译者:有些分支一支拖着不发布,最后与主干分支之间会相差很远)
有几个因素会导致评审拖拉。代码审查代码量大是一个因素,谁想审核涉及数十或数百个文件的更改?另一个因素是没有将代码评审作为真正的工作或可交付成果的一部分进行评估。困难或沮丧的代码审查体验是另一个主要因素:没有人想停止编码(开发人员通常喜欢的东西)来参与一个耗时和破坏灵魂的活动。
以下是解决审查拖拉的建议:

  • 确保代码评审量很小。每个团队都必须定出合适自己的大小,但它将在几个小时或几天的工作范围内进行审查,而不是几周。

  • 确保代码评审目的明确,并且评审人员应该明确要找什么。当范围是“发现代码的任何可能问题”时,很难激励自己去做一些事情

  • 在开发过程中留出时间进行代码评审。 最后一点可能需要团队纪律,或者团队可能希望将代码评审行为通过(例如)目标或任何用于确定开发人员生产力的机制来激励大家花时间评审代码。

你的团队能做什么?

专注于创建一个可靠的代码评审过程。我已经在我的博客上写过了,但是我想在这里分享这个过程的一部分。
在进行代码评审时,有很多事情需要考虑,如果开发人员在每次代码评审时都担心所有这些问题,那么任何代码都几乎不可能通过评审过程。对每个人都有效的代码评审最佳方法是考虑以下问题:

  • 为什么团队要做评审代码?当有明确定义的目的时,评审员的工作会更轻松,而代码作者从评审过程中得到的令人讨厌的惊喜也会更少。

  • 团队成员需要关注什么?当有目的时,开发人员可以创建一组更集中的东西来检查代码。(When there’s a purpose, developers can create a more focused set of things to check when reviewing code.)

  • 涉及到谁?谁负责审查,谁负责解决意见冲突,谁最终决定代码是否适合发布?

  • 团队什么时候进行评审,评审什么时候完成?在开发人员处理代码或在流程结束时,可以迭代地进行评审。如果没有明确的指导来确定代码何时适合使用,那么审查可能会一直进行下去。

  • 团队在哪里进行评审?代码评审不需要特定的工具,所以评审也可以简单的在作者办公桌上带同事浏览代码。 一旦回答了这些问题,您的团队就应该能创建一个运行良好的代码评审过程。记住,评审的目标应该是让代码投入生产,而不是证明开发人员有多聪明。

结论

通过一个清晰的代码评审过程,可以消除或至少减轻代码评审的误区。许多团队认为他们应该做代码评审,但是他们没有明确的指导方针来解释为什么要做代码评审。
不同团队需要不同类型的代码审查,就像不同的应用程序有不同的业务和性能需求一样。第一步是找出团队需要代码评审的原因,然后团队可以基于此进行评审。

  • 自动化简单检查(例如,检查代码样式、识别常见错误和查找安全问题)

  • 明确何时进行审查、关注什么以及由谁决定是否完成等审查指导方针

  • 使代码评审成为开发过程的关键部分 关注为什么需要代码评审将有助于团队为评审过程创建最佳实践,从而更容易避免代码评审误区。

数码
沪ICP备19006215号-4