草庐IT

什么?代码审查存在缺陷?我带你搞定它!

崔皓 2023-03-29 原文

​译者 | 崔皓

审校 | 孙淑娟

一、开篇

为了提升代码质量,需要将批判性思维带入到编程中去。因此,需要将工程方法应用到代码的审核过程。虽然,软件工程师,在讨论抽象类和函数时信心十足,但谈论"管理 "时,这种信心却荡然无存。

在整个编程过程中,由于各种原因会存在大量的缺陷,这就需要通过代码审查的方式将这些缺陷找出,才能保证软件质量。这篇文章将从不同的角度来看待代码审查,并提出改进的意见。

在《软件工程的事实与谬误》一书中,有这样的描述:“严格的检查可以在运行第一个测试用例之前消除软件产品中高达90%的错误。”

Bob 对代码审查的回复

虽然无法确定这话是针对代码审查的,但是可以理解为不同种类的检查确实对软件质量有帮助。1976年,Michael Fagan在他的文章《设计和代码检查以减少程序开发中的错误》中提出了代码检查的想法。

包括以下三种类型的检查:

  • 设计检查
  • 单元测试前的代码检查
  • 单元测试后的代码检查

Michael Fagan关于设计和规范检查的文章中的一个方案

Fagan的工作没有提出新的代码审查方法,而是记录了已经存在的现象,并为其进行论证。然而,这篇文章是最早的记录代码检查的书面作品。

代码检查(Code inspection)看起来像现代的代码审查(Code review)。那我们为什么今天会错过其他类型的检查?

二、为什么今天只有代码审查的假设

先进代码检查的流以及其他类型检查方式的销声匿迹,得益于我们使用的代码工具。例如:GitHub、BitBucket或GitLab它们都内置了代码检查的工具,并天然地适合Git flow、GitHub flow和其他方法。

在设计活动中使用什么工具?这与UI/UX无关,只和代码结构相关。你可能听说过CASE或UML工具。在我工作过的7家公司中,并没有看到它们被认真使用。

在HackerNoon上,关于"UML "的搜索查询结果只有两个。所以当没有解决方案设计过程时,并不需要介绍设计检查。在我领导的团队中,使用Miro进行界面设计。整个设计过程本很令人满意:和其他图表工具一样,你很快就开始画图,而不是解决设计方面的问题。我们要解决的问题和工具提供的功能被割裂了。下面是 《投资无限 》一书中的一小段话,可以支持这个观点。

“......如果我们只是做工具能做的事--那么我们将永远局限于工具的能力。”

三、现有的代码审查有什么缺陷?

让我们从不同的角度看如下几个处理过程,每一个都存在重大的问题。

1.BPMN透视

BPMN是业务流程建模标记系统的建成。它用动作、事件、逻辑网关、消息和其他手段来描述流程。如果你想开发一种算法,我甚至建议使用它,因为它比流程图更具有描述性。让我们用这个符号来描述代码审查过程,并对其进行分析。我使用了一个基于文本的工具来生成图表。

经典的代码审查流程图

一切从创建PR(Pull Request)开始,下一步是通知审查员,这是做了简化,可以说。"嘿,我的PR在等着你!这里需要等待,然后审查员进入任务,进行审查。有可能一个PR会马上被合并。当然,相反的情况也可能发生:会提出一些修正意见。此时代码的作者可能已经在做下一个任务了,那么就需要等待一段时间。当作者返回到有修改意见的任务时,就需要恢复上下文,解释注释并进行修复。

在修复之后,下一步就是通知审查员重新进行代码审查了。

这种情况仿佛似曾相识,是的,代码审查和修复是一个无限的循环,上面描述的仅仅是这个循环中的一次而已。审查员总能够发现新的问题,抛出新的修改建议。又或者整个循环会在程序作者的其他工作影响下,一直等待下去。

我们是否希望无限循环成为日常运作的一部分?我不确定,因为拥有更快的交付通常是我们所期待的。

2.解决问题的方式方法

有时,团队中的高级开发人员或架构师会担当审查员。他们希望有一致的代码库,同时还会坚持一些编程的方法和模式。也就是说审查员有自己一套想法(愿景)的,当然开发人员也有他们的想法。通常,两波人不知道对方的意图。这需要有一种方式将他们之间的观点和看法打通,有助于他们能够站在同一层面思考问题,但实际上这种情况很少发生。让我们看看下面的图片。

在经典的代码审查过程中,代码创建者和

审查者的观点随着时间的推移而出现分歧

可以看到代码审查参与者的两个思想观念是不同的。在第一次迭代之后,他们开始对齐,但仍有一段路要走。评审员调整一个人的视野,而代码作者则根据建议来行动。

就好像"你已经要了一栋房子,然后惊喜地发现!它不是你所期望的那个 "。想象一下,你已经要求一个人去实现一些事情。实现以后再看结果,让你非常惊讶。不要慌着惊讶,因为你并没有告诉执行者做这件事情的“决策框架”,才导致结果和你预期的存在偏差。

3.人际关系的角度

代码审查备忘录

这张图片本身就说明了问题,审查的代码量越少发现越多的问题,代码量越大反而“不会发现问题”。坦率地说,如果你是审查者,你会要求你的同事花费很长时间(好几天)彻底修复一个设计问题吗?特别是在迭代的冲刺阶段,在开发时间本来就非常紧张的情况下,会这么做吗?恐怕,你想得是快点完成功能,合并代码发布吧?!另一方面,如果存在代码修复需要修改系统中很多地方,你会做出修改的决定吗?

虽然,精益生产的思想并没有影响到编程。然而,在Mary Poppendieck和Tom Poppendieck的《精益软件开发》中就有对软件开发7大浪费的描述。它们包括:

  • 部分完成的工作
  • 额外功能
  • 重新学习
  • 交接
  • 延迟
  • 任务转换
  • 缺陷

下面,我们花一点篇幅对这7点进行展开说明:

部分完成的工作。审查代码没有得到足够的重视,审查只是提升代码质量的开始,而不是软件开发的结束。有一种有趣的心态:"开发完成了,审查任务提交了,剩下的就不是我的工作了!",这就导致了,代码审查是 "三不管"的地方,只有上级问起的时候才得到重视。

  • 重新学习。在BPMN图上看到重新学习的情况,程序员在收到修改建议时,手上可能有其他的任务。当着手对审查代码进行修改的时候,已经离完成该项任务有一段时间了,为了根据意见进行修改,就不得不对业务和技术背景进行回顾,这也就是重新学习。恢复业务和技术的上下文对程序员来说是有开发成本的。(这种情况对于审查者也适用)
  • 交接。代码的交接,修改建议的描述,中间存在团队成员的沟通,有沟通就存在损耗。
  • 延迟。代码审查设计包含我们前面讨论过的两种类型的延迟:修改本身的延迟和思想观念的延迟。
  • 任务转换。人们暂停他们的任务,对审查给予一些关注。
  • 缺陷。审查有利于发现表面问题,但不能发现设计缺陷,而设计缺陷会造成最大的伤害。上面提到的缺乏向研究员提出重大修改的动机,导致了项目中的大量缺陷。

我们已经从很多方面阐述了代码审查的问题。我们能做些什么来解决这些问题呢?

四、重新设计代码审查

在本节中,我们将针对上面提出的问题,看看如何对其进行优化。

1.修复流程结构

代码审查流程图

图中有代码作者在等待审查完成,以及在审查员的概述问题之后的结对编程会议。

在过程中,可以看到与之前过程相比,有几个显著的变化。

  • 不再有潜在的无限审查循环。
  • 在等待的未知时间内也会得到处理。
  • 不需要为代码作者恢复上下文或解释反馈。评论只是作为提醒。

这个过程发生的核心条件是什么?团队需要一个额外的角色。这意味着有人做一些辅助活动,例如:处理技术债务,或修复优先级较低的错误。当代码审查出现时,这个人就会立即放下当前的任务,进行PR工作。

2.修复观念差异

我们在代码审查中讨论的内容,在开发过程中做出的任何决定,无论何时何地都需要有同理心,都要站在对方的角度思考而不是从自身出发。

对于设计的决定需要在设计完成时就进行,而不要等到编码阶段。当然,这里需要一个额外的审查类型:设计审查。有时,问题具有挑战性,就需要花一些时间在计划上,与知识渊博的同事聊聊会有意外收获。

有一句著名的军事格言道:“没有任何作战计划能在与敌人的接触中幸存下来。”

如果把它翻译成系统语言,应该是:"当第一个反馈到来时,系统肯定需要进行调整"。在编程过程中,反馈就是将设计实施到系统中所面临的问题。因此,一些之前做出的决定需要修改的时候就应该被修改。在修改的过程中,会因为观念和愿景的差异再次与评审员产生分歧。

Adam Thornhill在他那本 《软件设计X射线》书中,提出了一种方法。

这就是为什么我建议你更早地进行初步的代码演练。与其等待一个功能的完成,不如在每一个功能完成三分之一的时候就进行介绍和讨论,这是一个惯例。少关注细节,多关注整体结构、依赖关系,以及设计与问题领域的一致性如何。当然,三分之一的完成度是主观的,但它应该是一个基本结构到位、问题被很好地理解、并且存在初始测试的节点。在这个早期阶段,重新设计仍然是一个可行的选择,抓住潜在问题会有很大的回报。

受到上面话的启发,我为我的团队创造了一句话:“架构式审查”。我希望它有助于反映活动背后的想法。

在代码审查过程中,代码创建者和审查者的愿景随着时间的推移而出现分歧。

3.精益视角下的推荐处理方式

代码审核的推荐处理方式可以消除或大大解决浪费行为。

部分完成的工作。在代码作者的预期时间内切换到另一个任务是不允许的,所以不存在用户意义上的部分完成的工作。优先级较低的部分完成的活动是权衡的结果。

再学习。由于在完成编码和结对编程之间的时间很少,所以不会发生重新学习。

延迟。我们已经大大缩短了代码审查员的延迟,消除了作者的延迟。

任务转换。对作者来说不再允许,而对审核者来说可以通过管理解决。

缺陷。现在,修复设计缺陷变得更加便利,其中最重要的设计缺陷也变得可以修复了。

五、补充思考

我们已经讨论了单个作者和单个审查员的代码审查方法和流程。当更多的审查者出现时,问题会成倍增加。

在试图引入推荐处理流程时,面临两个最具挑战性问题:

第一,开发人员把审查阶段当作一项工作来完成。当在日常工作中引入冗余人员会让经理们感到惊恐。解决这个问题的方法是适当的冗余和加快审核的速度。

第二个问题更加复杂。在这里我想引用丹尼尔-瓦坎蒂《可预测的敏捷指标》书中的一段话。

联邦快递采用了很多策略,但最重要的可能是,在任何时候联邦快递都有空飞机在空中。是的,我说的是空飞机。这样一来,如果一个地方被淹没了,或者如果包裹被遗弃了,如果安排的飞机已经满了,那么就会有一架空飞机被转到问题地点(应该说是及时的)。在任何时候,联邦快递都有 "备用飞机"!

如果你是一个经理,下次在规划利用率最大化时请考虑如下问题。(翻译者:这里是让经理们考虑在产出最高,上升成本最低的情况下,为了软件质量,需要有部分的备份和冗余)

  • 我们对这次更新满意吗?是的,它看起来比我们现在的情况要好。
  • 我们能做得更好吗?是的,我们可以。

如果目标能在保证质量的情况下达成,可以取消代码审查。为了实现这个目标,我们需要建立一个辅助决策框架,以帮助开发人员应用最佳实践。当然,我们从来没有听说过这样的框架,但in-IDE linters是向它迈出的一步。

原文链接:https://hackernoon.com/code-reviews-is-inherently-flawed-heres-how-to-fix-it

译者介绍

崔皓,51CTO社区编辑,资深架构师,拥有18年的软件开发和架构经验,10年分布式架构经验。

有关什么?代码审查存在缺陷?我带你搞定它!的更多相关文章

  1. ruby - 为什么我可以在 Ruby 中使用 Object#send 访问私有(private)/ protected 方法? - 2

    类classAprivatedeffooputs:fooendpublicdefbarputs:barendprivatedefzimputs:zimendprotecteddefdibputs:dibendendA的实例a=A.new测试a.foorescueputs:faila.barrescueputs:faila.zimrescueputs:faila.dibrescueputs:faila.gazrescueputs:fail测试输出failbarfailfailfail.发送测试[:foo,:bar,:zim,:dib,:gaz].each{|m|a.send(m)resc

  2. ruby-on-rails - Rails - 子类化模型的设计模式是什么? - 2

    我有一个模型:classItem项目有一个属性“商店”基于存储的值,我希望Item对象对特定方法具有不同的行为。Rails中是否有针对此的通用设计模式?如果方法中没有大的if-else语句,这是如何干净利落地完成的? 最佳答案 通常通过Single-TableInheritance. 关于ruby-on-rails-Rails-子类化模型的设计模式是什么?,我们在StackOverflow上找到一个类似的问题: https://stackoverflow.co

  3. ruby - 如何在 buildr 项目中使用 Ruby 代码? - 2

    如何在buildr项目中使用Ruby?我在很多不同的项目中使用过Ruby、JRuby、Java和Clojure。我目前正在使用我的标准Ruby开发一个模拟应用程序,我想尝试使用Clojure后端(我确实喜欢功能代码)以及JRubygui和测试套件。我还可以看到在未来的不同项目中使用Scala作为后端。我想我要为我的项目尝试一下buildr(http://buildr.apache.org/),但我注意到buildr似乎没有设置为在项目中使用JRuby代码本身!这看起来有点傻,因为该工具旨在统一通用的JVM语言并且是在ruby中构建的。除了将输出的jar包含在一个独特的、仅限ruby​​

  4. ruby - 什么是填充的 Base64 编码字符串以及如何在 ruby​​ 中生成它们? - 2

    我正在使用的第三方API的文档状态:"[O]urAPIonlyacceptspaddedBase64encodedstrings."什么是“填充的Base64编码字符串”以及如何在Ruby中生成它们。下面的代码是我第一次尝试创建转换为Base64的JSON格式数据。xa=Base64.encode64(a.to_json) 最佳答案 他们说的padding其实就是Base64本身的一部分。它是末尾的“=”和“==”。Base64将3个字节的数据包编码为4个编码字符。所以如果你的输入数据有长度n和n%3=1=>"=="末尾用于填充n%

  5. ruby - 解析 RDFa、微数据等的最佳方式是什么,使用统一的模式/词汇(例如 schema.org)存储和显示信息 - 2

    我主要使用Ruby来执行此操作,但到目前为止我的攻击计划如下:使用gemsrdf、rdf-rdfa和rdf-microdata或mida来解析给定任何URI的数据。我认为最好映射到像schema.org这样的统一模式,例如使用这个yaml文件,它试图描述数据词汇表和opengraph到schema.org之间的转换:#SchemaXtoschema.orgconversion#data-vocabularyDV:name:namestreet-address:streetAddressregion:addressRegionlocality:addressLocalityphoto:i

  6. ruby-on-rails - Rails 源代码 : initialize hash in a weird way? - 2

    在rails源中:https://github.com/rails/rails/blob/master/activesupport/lib/active_support/lazy_load_hooks.rb可以看到以下内容@load_hooks=Hash.new{|h,k|h[k]=[]}在IRB中,它只是初始化一个空哈希。和做有什么区别@load_hooks=Hash.new 最佳答案 查看rubydocumentationforHashnew→new_hashclicktotogglesourcenew(obj)→new_has

  7. ruby - 为什么 4.1%2 使用 Ruby 返回 0.0999999999999996?但是 4.2%2==0.2 - 2

    为什么4.1%2返回0.0999999999999996?但是4.2%2==0.2。 最佳答案 参见此处:WhatEveryProgrammerShouldKnowAboutFloating-PointArithmetic实数是无限的。计算机使用的位数有限(今天是32位、64位)。因此计算机进行的浮点运算不能代表所有的实数。0.1是这些数字之一。请注意,这不是与Ruby相关的问题,而是与所有编程语言相关的问题,因为它来自计算机表示实数的方式。 关于ruby-为什么4.1%2使用Ruby返

  8. ruby - ruby 中的 TOPLEVEL_BINDING 是什么? - 2

    它不等于主线程的binding,这个toplevel作用域是什么?此作用域与主线程中的binding有何不同?>ruby-e'putsTOPLEVEL_BINDING===binding'false 最佳答案 事实是,TOPLEVEL_BINDING始终引用Binding的预定义全局实例,而Kernel#binding创建的新实例>Binding每次封装当前执行上下文。在顶层,它们都包含相同的绑定(bind),但它们不是同一个对象,您无法使用==或===测试它们的绑定(bind)相等性。putsTOPLEVEL_BINDINGput

  9. ruby - Infinity 和 NaN 的类型是什么? - 2

    我可以得到Infinity和NaNn=9.0/0#=>Infinityn.class#=>Floatm=0/0.0#=>NaNm.class#=>Float但是当我想直接访问Infinity或NaN时:Infinity#=>uninitializedconstantInfinity(NameError)NaN#=>uninitializedconstantNaN(NameError)什么是Infinity和NaN?它们是对象、关键字还是其他东西? 最佳答案 您看到打印为Infinity和NaN的只是Float类的两个特殊实例的字符串

  10. ruby-on-rails - 如果 Object::try 被发送到一个 nil 对象,为什么它会起作用? - 2

    如果您尝试在Ruby中的nil对象上调用方法,则会出现NoMethodError异常并显示消息:"undefinedmethod‘...’fornil:NilClass"然而,有一个tryRails中的方法,如果它被发送到一个nil对象,它只返回nil:require'rubygems'require'active_support/all'nil.try(:nonexisting_method)#noNoMethodErrorexceptionanymore那么try如何在内部工作以防止该异常? 最佳答案 像Ruby中的所有其他对象

随机推荐