代码评审【未整理】
代码评审
代码评审也称代码复查,是指通过阅读代码来检查源代码与编码标准的符合性以及代码质量的活动。
代码评审应该与团队的现有流程集成。例如,如果一个团队正在使用任务分支工作流,那么在所有代码编写完成并通过自动化测试之后,在代码合并之前,就会启动代码评审。这确保了代码评审人员的时间被用来检查机器遗漏的东西,并防止糟糕的编码决策污染开发主线。
代码评审是指在软件开发过程中,对源代码的系统性检查。通常的目的是查找系统缺陷,保证软件总体质量和提高开发者自身水平。 Code Review是轻量级代码评审,相对于正式代码评审,轻量级代码评审所需要的各种成本要明显低的多,如果流程正确,它可以起到更加积极的效果。正因如此,轻量级代码评审经常性得被引入到软件开发过程中。
一、评审的内容
-
通过工具来进行code review不在本次讨论范围内。
-
编码规范问题:命名不规范、magic number、 System.out……
-
代码结构问题:重复代码、巨大的方法和类、分层不当、紧耦合
-
工具、框架使用不当:Spring、Hibernate、AJAX
-
实现问题:错误验证、异常处理、事务划分、线程、性能、安全、实现过于复杂、代码可读性不佳、扩展性不好
-
测试问题:测试覆盖度不够、可测试性不好
-
代码评审不负责检查功能、逻辑是否正确,这些要靠单元测试和QA工作来解决
-
在代码中是否存在明显的逻辑错误?
-
查看需求,是否所有用例都实现了?
-
新的自动化测试是否足以满足新代码?
-
是否需要重新编写现有的自动化测试用例来应对代码的变化?
-
新代码是否符合现有的开发规范?
二、评审的类型
一般来说,代码评审分为正式代码评审与轻量级代码评审两种
Formal Code Review(正式代码评审)
Fagan inspection(范根检查法):
-
RolesAuthor/Designer/Coder: 作者
-
Reader: paraphrases the document(阅读者)
-
Tester: reviews the document from a testing standpoint(评审员)
-
Moderator: responsible for the inspection session, functions as a coach(协调人)
-
Recorder:record detects.(记录员)
Lightweight Code Review(轻量级代码评审)
几种常见的轻量级代码评审方式:
-
Over-the-shoulder:它由作者启动和主持评审,作者向评审者展示文档。优点是启动快,成本低,缺点是容易被作者误导过程)
-
Email pass-around:优点自动化,可以及时提供最新代码进行评审,缺点是无法达到人工筛选的功效)
-
Pair Programming:源于XP,作者与评审者平级,可以帮助同伴间的学习和共享)
-
Review Meeting:定期组织review会议,轮流有团队成员选出自己的评审作品,需要系统化得预备、总结和追踪。优点可以提高团队整体技能和对产品的理解,缺点是评审范围有限,成本较高
-
Tool-assisted code review:大量的代码评审工具,比较流行的checkstyle/findbugs/pmd
三、代码评审的选择
-
最近一次迭代开发的代码
-
系统关键模块
-
业务较复杂的模块
-
缺陷率较高的模块
四、评审的好处
-
提高代码质量
-
在项目的早期发现缺陷,将损失降至最低
-
评审的过程也是重新梳理思路的过程,双方都加深了对系统的理解
-
促进团队沟通、促进知识共享、共同提高
-
交叉评审——代码走查:团队成员互相检查代码
-
参与者可以是任意两个组员,或开发组长分别与每个组员结对进行
-
时机可以选择在下班前半小时,对当天改动的模块进行评审
-
代码作者讲解如何以及为何这样实现、评审者提出问题和建议
-
每次解决的问题要记录到SVN注释或JIRA
-
每次评审不要贪多,如下图所示:当一次评审超过400行代码时,能发现缺陷数显著降低——事倍功半
-
会审:以项目为单位,召开专门的代码评审会议
-
参与者:包括项目组全体成员,其它组的开发组长也应尽量参加
-
时机选择:开发进行到某一阶段时,对共性问题进行总结,对好的做法进行提炼和推广
-
提高质量
-
及早发现潜在缺陷与BUG,降低事故成本。
-
促进团队内部知识共享,提高团队整体水平
-
评审过程对于评审人员来说,也是一种思路重构的过程。帮助更多的人理解系统
代码评审共享知识
无论开发方法如何,每个团队都可以从代码评审中获益,敏捷团队更是能获得巨大的好处。因为团队的工作是分散的,通过代码评审可以做到没有人是唯一知道代码库特定部分的人。简单地说,代码评审有助于促进跨代码库和整个团队的知识共享。
所有敏捷团队的核心都是战无不胜的灵活性:一种将工作从待办事项列表中划掉并由所有团队成员开始执行的能力。因此,团队能够更好地围绕新工作进行展开,没有人是“关键路径”。全栈工程师可以处理前端工作和服务器端工作。
随着代码评审使开发人员接触到新的想法和技术,他们会编写出更好的代码。
通过代码评审可以更好的进行工作评估
还记得评估的那一节吗?评估是一项团队练习,当产品知识在团队中传播时,团队会做出更好的评估。随着新特性被添加到现有代码中,原开发人员可以提供良好的反馈和评估。此外,任何代码评审人员也会综合考虑代码库的复杂性、已知的关注的问题。通过这种方式代码评审员分享了代码库中那个部分的原开发人员的知识。这种实践使得产品知识有多人了解,当大家做最终评估时,通常会使评估更加可靠。
代码评审能让你享受休假
没有人喜欢成为一段代码的唯一联系人。同样地,没有人愿意钻研不是他自己写的关键代码——尤其是在生产环境有紧急情况发生时。代码评审在整个团队中共享知识,这样任何团队成员都可以接管并继续领航这艘大船。(我们喜欢在 Atlassian 进行这样的比喻!)但这里的重点是:没有任何一个开发人员是关键路径,这也意味着团队成员可以根据需要休假。如果你发现自己在产品开发中忙的焦头烂额时,代码评审是一种很好的方式让你获得自由。自由地去享受个假期,或者自由地去看看产品其他领域的事情。
通过代码评审指导新工程师
敏捷开发的一个特点是当新成员加入团队时,经验丰富的工程师会指导新成员。代码评审有助于促进关于代码库的沟通。通常,团队在代码评审期间的代码中隐藏了产品知识。新成员带着新鲜的眼光,从新视角来审查代码库的粗糙之处和历史遗留缺陷的地方。因此,代码评审也有助于确保新见解与现有知识相调和。
专业提示:请记住,代码评审不仅仅是高级团队成员评审初级团队成员的代码。代码评审应该在各个方向上进行。知识是没有界限的!代码评审可以帮助新加入的工程师,但绝不应该仅仅作为一种指导练习。
五、会前准备工作
-
组织者应通知各参与者本次评审的范围
-
参与者阅读源代码,列出发现的问题、亮点,汇总给组织者
-
准备工作要细致,需要给出问题详细描述以及相关代码在SVN上的URL地址等
-
评审代码的选择:
-
最近一次迭代开发的代码
-
系统关键模块
-
业务较复杂的模块
-
缺陷率较高的模块
六、会议议程
-
如果是第一次会议,先由该项目开发组长做整体介绍
-
参加者依次发言,结合代码讲解发现的问题
-
每讲完一个问题,针对其展开讨论,每个问题控制在10分钟以内
-
如果问题不多,还可以安排该组成员对最近开发的代码进行地毯式的讲解和排查;或者针对某个方面对整个项目做评审,例如性能、安全性或测试
七、会后总结
-
把会上提出的所有问题、亮点及最终结论详细的记录下来,供其他团队借鉴
-
未能讨论清楚的问题,会后解决
-
实行代码评审制度前的准备工作:
-
架构师提供开发规范、指南,为代码评审提供依据
-
建立起单元测试规范,否则无法达到测试覆盖度的要求、难以修正发现的问题
-
最好有样例代码库作参照,以提高代码评审的可操作性
-
提供评审案例:用评审前的代码与评审后优化的代码做对比
-
问题跟踪:对评审中发现的问题代码应加以跟踪,确保问题得以解决,防止复发
八、评审到什么程度
-
进行全面的代码评审成本较高,也没有必要
-
对发现的问题要本着集体代码所有制的观点和就事论事的原则,因此建议把代码质量与团队绩效(而不是个人绩效)挂钩
九、实践要素
- 人员结构
在你的团队中有多少同事拥有足够的专业技能来担当合格的代码审查者?作者曾经和多个开发团队沟通过,这些团队都声称本身只有一个资深的开发人员,如果让他来负责所有的代码审查工作,那么团队的核心开发就无法开展了。作者也遇到过类似的情况。在一个小规模团队里,资深的同事总是忙不过来,因为核心的工作都在等着他做。
- 地理位置
你的团队成员是如何分布的?他们是否是分散在世界各地还是坐在同一个敞亮的办公室里? 代码审查需要精力的专注和反馈,如果开发人员能够面对面的沟通,那么审查工作会便于实施。而物理隔离的远程团队很难达到代码审查的满意结果。
- 所在领域
你所在的IT领域需要遵守怎样的规则和约束呢?如果你在一个受到严格监管的行业,那么你必须遵循有关审计和报告的规则,这意味着你必须想办法跟踪代码审查的频率和质量。如果所在的领域把安全性作为重点,那么可能在代码审查过程中要引入安全审计。
- 复杂性
你的代码复杂性如何?在代码审查时,需要多个不同专业技能的审查者吗?例如,游戏开发可能会引入非常复杂的业务逻辑来处理文字交互和场景跟踪,同时还需要特定的动画处理和内存管理技术。在审查如此复杂的代码时,应该引入多个审查者从不同角度来检阅代码的质量。
代码评审不是批斗会,不能以缺陷和错误来打击开发人员的积极性评审的目标的提高质量和提高整体水平,作者应该带着学习和提高的态度来参加评审。
代码集体所有制:对发现的问题要本着整体承担责任 的原则,因此建议把代码质量与团队绩效(而不是个人绩效)挂钩。
评审程度,进行一次整体的地毯式的评审成本很高。
代码评审的可操作性,首先需要评审团队具备经验丰富的系统架构师和精通业务的行业专家。其次团队需建立其开发规范或指南,在项目初期建立少量的Sample代码与checklist为评审提供依据。
评审人员的职责是发现工作成果中的缺陷,并帮助开发人员给出消除缺陷的办法,而不是替开发人员消除缺陷 。
记录评审中出现的问题,跟踪改进。
评审前充分准备,评审后详细总结。
不要因为时间和成本问题取消评审。
image.png在“上古”时代,代码作者在开始 Code Review 前,还需要手动做一份变更列表(changelist),来告诉评审者这一次提交做了什么改动。得益于 Git 的诞生,今天的我们可以借助基于 Git 版本控制系统的平台,来更轻松无痛地开展代码审阅,比如「CODING 企业版」,作为企业级软件研发管理系统,其提供的 Code Review 功能简单好用,能大大提高代码审阅效率:
image.png借助 Git 自动实现精细的文件改动,红色代表删减,绿色代表新增,支持行级评论,再也不用在不同工位间来回走动,直接在具体代码下进行交流。
长远来看,代码评审可以节省时间
代码评审确实有点耗时,但是“做评审所花费的时间”不会比“不做评审所浪费的时间”多到哪里去。
只要执行得好,从长远来看,代码评审是可以节省团队时间的。原因有如下三点:
代码评审可以分担任务负担
很多 Atlassian 的团队,需要对任何代码进行双重评审才能合并到代码库上去。听起来是不是有点额外增加工作负担了?但实际上不会。当代码作者递交评审时,是递交给整个团队的。任何工程师都可以加入评审。这使得任务负担可以被分担,从而可以防止“一项工作耽搁影响总体进度”的情况发生。也可以保证团队代码评审的质量。
代码评审可以把软件bug扼杀在摇篮里
把代码评审作为提交代码前的必要工序去执行,可以让很多问题(例如熬夜做了不靠谱的架构决策、实习生用了不合理的设计模式)在软件最终发布前就被发现。
代码评审可以提高工作质量
当一个工程师知道自己的代码会被同事审阅时,他会倾向于付出更多努力来让自己的程序设计变得足够优雅、让自己的代码能顺利通过全部测试。工程师们有了这个上进意识,最终也可以让整个项目变得更加顺畅、有效。
在开发周期里,如果急着得到反馈,那就不要等代码评审给出意见。提前得到反馈,往往会产出更好的代码。因此,无论何时,都不要羞于让别人提意见。这会提高你后续的工作质量,也会提高整个团队的代码评审能力。从而让开发团队进入正循环。
<u>https://www.atlassian.com/agile/software-development/code-reviews</u>
四种主流的代码审查(code review)类型
每个专业的软件开发者都知道,代码审查是任何正式开发过程中的必要环节。但大多数开发者不知道的是,代码审查分为很多种类型。根据你项目和团队架构的不同,每一种代码审查类型都有它特有的优缺点。
首先,在一个很高的层面,你可以将代码审查归为两大类:正式的代码审查(formal code review),和轻量级的代码审查(light weight code review)。
正式的代码审查
正式的代码审查是基于正式的开发流程。其中最流行的实践是范根检查法(Fagan inspection)。
它为试图寻找代码的缺陷提供了一种非常结构化的流程,并且,它还可以用于发现规范(specifications)中的或者设计中的缺陷。
范根检查法由6个步骤组成:计划(Planning),概述(Overview),准备(Preparation),召开检查会议(Inspection Meeting),重做(Rework),和追查(Follow-up)。基本的思想是:预先制定好每一个步骤所需要达到的输出要求。接下来,当进行到某个过程时,你检查其现在的输出,并与之前制定的理想输出要求做比较。然后,你由此来决定,是否进入下一个步骤,或者仍需在当前步骤继续工作。
这种结构化的流程用的并不多。事实上,在我的职业生涯中,我从没遇到过哪一个团队使用这种方法,而且我也不认为我能在将来看到这种情况。
我认为其原因是,这种流程带来很大的开销,并没有多少团队用到它。
然而,如果你开发的软件生死攸关,会因为有缺陷而让人丧命,那么以这种结构化的方式去查找软件缺陷就显得很合理了。
例如,你是为核电站开发软件。你可能需要一个非常正式的流程去保证最终交出去的代码是没有问题的。
但像我所说,我们大部分开发者所做的软件都不是危及生命的,因此我们使用一种更加轻量的代码审查方法作为正式流程的替代。
所以,让我们来看看这种轻量级的方法。
轻量级的代码审查
如今,轻量级的代码审查在开发团队中很常用。
你可以将轻量级的代码审查细分为不同的子类:
瞬时的代码审查,也称为结对编程(pair programming);
同步的代码审查,也称为即时(over-the-shoulder)代码审查;
异步的代码审查,也称为有工具支持的(tool-assisted)代码审查;
偶尔的代码审查,也称为基于会议的(meeting-based)的代码审查。
类型1:瞬时的代码审查
第一种类型是瞬时代码审查,它发生在结对编程的情景中。当一个开发者在敲键盘写代码的同时,另一个开发者盯着代码,注意着代码中潜在的问题,并在此过程中给出提升代码质量的建议。
复杂的业务问题
当你需要解决一个复杂问题时,这种代码审查的方法很适用。在仔细寻找解决方案的过程中,把两个人的脑力聚集起来,会增加成功的几率。让两个头脑思考同一个问题,并相互讨论可行的方案,这样你会更可能覆盖到问题的一些边界情况。
在遇到需要很多复杂业务逻辑的任务时,我喜欢使用结对编程。这样,有助于两个人彻底理清流程中的所有不同的可能性,保证所有情况都在代码中得到了适当的处理。
与复杂的业务逻辑不同,有时,你也会需要去解决一个复杂的技术问题。例如,你在使用一个新的框架,或者在探索之前你没用过的一些新技术。在那种情况下,最好还是单独行动,因为你可以根据自己的情况作出快速调整。为了弄清新技术是如何工作的,你需要上网搜索大量资料,或者阅读文档。
这时,结对编程的帮助就不大了,因为你们会成为各自获取这些知识的阻碍。
然而,当你被问题卡住之后,与你的同事交流一下解决方案,往往会帮你获得看问题的不同视角。
相同的专业水平
考虑进行结对编程的另一个重要方面,是一起工作时,两个开发者的专业水平。两个开发者最好是处于同一水平,因为这样他们才能以相同的速度一起工作。
让一个初级开发者和一个高级开发者进行结对编程,效果并不好。在初级开发者负责写代码的时候,坐在旁边的高级程序员可能会因为他写得太慢了而感到烦恼。如此设定,这个高级程序员的能力就被限制住了,从而浪费了时间。
而当键盘在高级程序员手上时,他又敲得太快,初级程序员跟不上高级程序员的思路。几分钟后,初级程序员就迷失在代码上下文里了。
只有当高级程序员慢下来,向初级程序员解释清楚他的做法,这种设定才合理。然而,这就不是我们所说的结对编程了,而是一种学习的环节,其中高级程序员在教初级程序员如何解决特定问题。
但是,如果两个开发者都在同一水平,在这种设定下,他们所能取得的进展是令人惊讶的。其中有一个很大的好处是,两个开发者能相互激励,当其中一位失去注意力时,另一位开发者能把他拉回正轨。
总结一下:结对编程适用于两个有相似经验水平的开发者处理复杂的业务问题的情况。
类型2:同步的代码审查
第二种类型是同步的代码审查。这种是,一个开发者独自编写代码,当她写完代码后,立即找代码审查者进行审查。
审查者来到开发者的桌前,看着同一块屏幕,一起审查、讨论和改进代码。
审查者不清楚代码的目标
当审查者不清楚这个任务的目标时,这种代码审查类型会很有效果。它会在这种情况下发生:团队里没有优化会议(refinement sessions),或者sprint计划会议(sprint planning sessions),来预先讨论每一项任务。
此做法通常会导致一个结果:只有特定的开发人员才知道某项任务的需求。
这样的情况下,在代码审查之前,向审查者介绍一下任务的目标是很有帮助的。
期待大量的代码改进
如果代码编写者缺乏经验,写出的代码需要很大的改进,那么同步代码审查也会很有效。
如果一个经验丰富的高级开发者将要对一个很初级的程序员写出的一段代码进行审查,那么,当初级程序员写完代码后就和高级开发者一起改进这段代码,效率是远远高于初级程序员自己一个人看的。
强行切换思路的缺点
但是同步审查有一大缺点,就是它强行切换了审查者的思路。它不仅让审查者感到沮丧,也拖慢了整个团队的效率。
类型3:异步代码审查
然后我们有了第三种类型,异步代码审查。这一类型的审查不是在同一时间、同一块屏幕上完成的,而是异步的。开发者在写完代码后,让这些代码对审查者可见,然后开始她的下一个任务。
当审查者有时间了,他会在自己的桌子上按自己的时间表进行代码审查。他而不需要当面和开发者沟通,而是用工具写一些评论。在完成审查后,那些工具会把评论和需要的改动通知给开发者。开发者就会根据评论改进代码,同样的,是以自己的时间表来做这些事情。
这个循环,会以代码改动再次被提交到审查者这里而又重新开始。开发者修改代码,直到没有评论说需要改进。最后,改动得到同意,并提交到主分支(master branch)。
你可以看到,同步的代码审查和异步的代码审查相比有很大的不同。
没有直接的依赖
异步代码审查的一大好处, 就是它是异步发生的。开发者不需要直接依赖于审查者,并且他们都可以按自己的时间表去做各自的工作。
多次审查循环的缺点
这里的缺点就是,你可能会有许多次循环的审查,它们可能会持续好几天,直到最终被接受。
当开发者完成代码后,通常需要几个小时,审查者才开始做代码审查。很多时候,审查者给出的建议只有在第二天才能被开发者修复。
这样,第一次审查周期就至少用掉了一天。如果你又多次这样的循环,审查的时间就延续至一整周了——这还不算写代码和测试的时间。
但这里有一些做法,可以避免这样的长时间间隔导致的失控。例如,在我的团队里,我们规定,每天上午,每个开发者在开始做其他工作之前,都要先处理积压的代码审查任务。同样的,在中午午休结束后也需要这样做。
因为在较长的休息时间后,开发者已经不处在他的代码思路中了。这时进行代码审查,你并没有强制它们进行不自然的思路切换,并且能够让代码在合适的时间得到审查。
对比这种代码审查类型的优缺点,我认为,异步的代码审查应该作为每一个专业开发团队的默认选项。
但在我告诉你为什么我是这么想的之前,让我看看第四种代码审查类型。
类型4:偶尔的代码审查
很久以前,我曾经每个月会和整个团队开一次代码审查会议。我们坐在会议室,一个开发者展示并解释着他最近写的一段困难的代码。
其他开发者尝试寻找着潜在的缺陷,发表评论,给出如何改进代码的建议。
我不认为任何团队和长期地使用偶尔代码审查的方式。我只想到这个类型适用于的一种情况:当整个团队都没有代码审查的经验时,让把每个人聚起来,一起做代码审查,这样弄几次之后,可能会帮助每个人理解代码审查的目标和意义。
然而,从长远来看,这第四种类型并不是一个合适的技术,因为让全组成员审查一段代码是很低效率的做法。
我应该选择哪种代码审查类型呢?
好了,现在你可能会想,该选哪种类型。
我们讨论了正式的类型,它显然不太流行,并且较难用于实践。
然后,我们讨论了轻量级的代码审查这一大类,然后是其中著名的4个子类型。
类型1,瞬时的代码审查,用于结对编程。当两个开发者有相似的技术组合,并且处理一些复杂的业务问题时,这种方式工作得很好。
类型2,同步的代码审查,用于审查者不清楚任务的目标时,需要开发者向其进行解释的这种情况。当开发者经验不足,写出的代码需要大量改进时,这种代码审查模式也工作得很好。
但是它的缺点是需要强行切换思路,会让审查者沮丧,以及拖慢团队开发速度。
类型3,异步的代码审查,避免了强行切换思路带来的问题,对大多数用例都工作得很好。
类型4,偶尔的代码审查,对于专业团队来说不是一个长期的选择。可以只在团队刚刚开始代码审查时被使用。
使用异步代码审查作为默认选择
我认为,专业的团队应该把异步的代码审查作为默认的选择。因为它避免了同步代码审查的缺陷。
当审查者不能理解开发者做出一项代码修改的原因时,可以使用同步的代码审查。但在那种情况下,审查者将会去询问开发者,以获得额外的信息和说明。如果你在一个团队中工作,这样的情况应该很少发生。
如果你不在一个真正的团队中,而是和一群人一起工作,那么同步的代码审查就有意义了。如果审查者对你过去这几天的工作内容毫不知情,那么在开始一起做代码审查之前,向审查者给出一个合适的说明是很合理的。
如果你有两个开发者,他们具备相似的技能组合,并且在攻克一个复杂的业务问题,那么也有理由切换到结对编程的模式。但是,一个团队往往由许多经验水平不同的成员组成,并且不会一直都在处理复杂的业务问题。大多数时间,你手上是复杂度在平均水平的常规任务。
因此,专业团队的最佳选择是:使用异步的代码审查作为默认选择,然后当需要时切换到同步的代码审查或者结对编程。
<u>https://blog.csdn.net/powertoolsteam/article/details/81699365</u>