代码评审的艺术——大牛来谈现实中的代码评审

简介:   曾经写过一点关于代码评审(code review)的文章,现在觉得关于它的认识又有了不少更新。软件工程的技术和实践分成两部分,一部分是和书本知识一致的,大约占一半,这部分基本上在大学里就可以学,自学只要方法得当、刻苦努力也可是途径;但是第二部分来自于实际团队、经验,内容通常无法从书本当中获得,而且难说对错,不同的人和不同的经历造就了不同的认识。

  曾经写过一点关于代码评审(code review)的文章,现在觉得关于它的认识又有了不少更新。软件工程的技术和实践分成两部分,一部分是和书本知识一致的,大约占一半,这部分基本上在大学里就可以学,自学只要方法得当、刻苦努力也可是途径;但是第二部分来自于实际团队、经验,内容通常无法从书本当中获得,而且难说对错,不同的人和不同的经历造就了不同的认识。代码评审就是第二部分颇具槽点,可以大加讨论的典型。

  代码评审是展现个性和性格的途径

  我本人特别反对一种颇为常见的观点,就是“一个良好运作的项目,不同的人,应该写出一样的代码”。我非常理解这种观点的初衷,一个良好规范约束的团队中,不同的人写出来的代码应当一致。但实际上,能真正这样做的团队,我还根本没有见到过。所谓的一致代码,仔细品味,发现不同的工程师写出来就是不一致。因此“一致”这个词一定是在一定程度内的大体描述而已,并非越一致约好。其实,代码的创造本就是具备个性的。毫无疑问我们应当遵从规约,应当符合习惯,但是一旦试图过度使用它们去约束代码,不但难以落实,而且容易产生无趣、低效和矛盾的氛围。

  再回到代码评审上。代码评审本身,以及基于评审意见的来回沟通,和代码本身比起来,要更难以,也更不应该要求一致。我见过许多代码评审的风格,有人喜欢挑小毛病,有人喜欢展开观点夸夸其谈,有人喜欢给实际例子来证明自己的看法……无论哪一种风格,我都不觉得有什么大的问题。但是,就我个人而言,我可以谈谈我自己的代码评审风格:

  我会关注三种问题,需求和业务上的问题、代码结构的问题、代码风格格式的问题。

  前两种可能存在阻碍我ship代码的“严重”问题(说“ship”就意味着认可代码具备了push到主线分支的条件了)。我已经记不清多少次和代码作者因为这样的问题争论了。争论是个艺术,有时候并不一定能够达成一致,有的人比较容易被说服,有的人则更坚持己见。我不想说哪一种更好,但是确实这是代码评审中展现风格的事实——都是就事论事,但有人害怕或者不喜欢得罪人,就会显得push over一点;有人则不在乎那么多,坚信自己的想法更合适,就会显得defensive一点。我可能属于后者,似乎在职业生涯的各种阶段都会有和我出现代码评审冲突的事例,但是在某些情况下我也会选择disagree and commit(不同意但是执行团队达成的意见)。我相信有些团队会喜欢我的backbone,也会有团队讨厌我的这种风格。下面的内容,也多为基于自己风格的表述。

  坚定自己的意见,但是委婉地表达

  关于这一点,我也在努力地改进。可以提及的点很多,技巧也很多,但是老实说,冲突还是不可避免。在我经历的几乎所有的团队中,有时候会有老好人,但是基本上所有的老好人都缺少对于原则的坚持。沟通是门艺术,在代码评审中也一样体现。

  • 最重要的一条,只针对代码,不针对人。这条很简单,都知道对事不对人的重要性,但是要非常小心不能违背。
  • 对于大多数代码风格格式上的问题,我都会标注这个问题是一个picky或者nit的问题(“挑剔的问题”,这是我在Amazon的时候学到的方式)。这样的好处在于,明确告知对方,我虽然提出了这个问题,但是它没有什么大不了的,如果你坚持不改,我也不打算说服你。或者说,我对这个问题持有不同的看法,但是我也并不坚信我的提议更好。
  • 使用也许、或许、可能、似乎这样表示不确定的语气词(包括使用虚拟语气)。这样的好处是,缓和自己观点表达的语气。比如:“这个地方重构一下,去掉这个循环,也许会更好”。
  • 间接地表达质疑。比如说,看到对方用了一个参数3,但是你觉得不对,但又不很确定,可以这样说:“我有一个疑问,为什么这里要使用3而不是其他值?”对方可能会反应过来这个值选取得不够恰当。
  • 放上例子、讨论的链接,以及其它一些辅助材料证明自己的观点,但是不要直接表述观点,让对方来确认这个观点。比如:“下面的讨论是关于这个逻辑的另一种实现方式,不知你觉得是否会稍简洁一些?”
  • 先肯定,再否定。这个我想很多人一直都在用,先摆事实诚恳地说一些同意和正面的话,然后用however一转,说出相反的情况,这样也就在言论中比较了pros和cons,意味着这是经过trade-off得出的结论。
  • ……

  一些很常见的可以在代码评审中提及的问题

  这样的问题其实有不少,多数和实现的技术无关,但是又很容易不小心略过。它们多数时候是问题,当然也有时候不是。

  • 比如说,我最痛恨的之一,职责过于宽泛或者职责不清的类或模块。我无数次见过这样的类:Helper,或者Utils(注意,它们都没有模型或者模块前缀)。考虑项目的规模,大多数情况下,这样的类很容易变成一个越吹越大的气球,似乎什么东西都可以往里搁,是个十足的违反单一职责原则的糟糕设计。
  • 比如说,在线程使用,容器使用,连接管理……中,缺乏上限控制的设计,在一些情况下导致资源使用过度膨胀。生产者和消费者的队列设计,一旦消费者挂掉或者跟不上,队列里越堆越多,没有拒绝机制。
  • 再比如说,缺少分包、分层,所有的东西都叠在一起,十几个,甚至几十个类文件并列在同一个文件夹下面。
  • 再再比如说,代码缺乏设计,流水账结构,面条型代码,或者简单铺陈几个过程式的方法,这种修改往往代价还不小,自然修改的落实率低,因而令提出问题的人也颇为头疼。
  • ……

  避免一次评审提及过多的问题

  少数情况下,手里拿到一份实在是问题多到令人发指的代码,这时候需要扼制自己想骂人的冲动。先抓问题的主干,不要去挑那些细枝末节的毛病,因为很有可能,这些代码会被改得面目全非,或者重写。写一大堆评审意见,反而容易淹没最重要的问题。

  说说容易,但是这个度有时候并不好掌握。我的习惯是,在明确代码要解决的问题以后,先快速走读一遍代码,判断我是应该粗略地抓主要矛盾呢,还是应该严格地挑刺呢。我也见过一些别的方式。

  不一样的评审要求

  我始终觉得,代码评审的要求不应该完全一致。不要过度追求公平。对于新项目代码,以及新员工写的代码,要适当严格一些。

  新项目的代码是形成后续风格和质量的模板。我们也许都听说过“破窗户原理”,在一块干干净净的代码田地里,新耕种的代码才容易保持一致,也可以避免一些“为了和现有代码风格保持一致”而导致质量妥协的情况出现。

  新员工代码的高质量要求则更多地在于对他们良好习惯养成的负责。软件工程师良好职业习惯的养成,代码可以说是最重要一环,而前三个月的影响举足轻重。

  还不如不做的评审

  有些情况下,代码评审是非常耗时费力的工作。特别是对业务背景不熟悉,对实现技术不熟悉,或者干脆是对方提交上来一大堆修改,阅读非常费力。我不知道哪一种是最难的,但是如果因为这些原因很难做到良好的评审,我会在评审中说明,或者放弃一些评审的工作,保证评审的质量。

  代码评审是建立在团队中影响的好方式。一方面是业务逻辑,一方面是代码结构,我反对在两方面都难以给出足够清晰的评审意见的时候,提一堆风格方面的次要问题。否则很容易达成负面影响。

  先谈到这里,我想还有不少可以涉及的内容,而它们都难以在课堂和书本上学到。争论才有意思,实践出真知大抵如此吧。你有什么一致的看法,或者反驳,欢迎和我讨论。

欢迎加入Java高级架构学习交流群:468947140

本群提供免费的学习指导 架构资料 以及免费的解答

进群修改群备注:开发年限-地区-经验

点击链接加入群聊【Java-BATJ企业级资深架构】:https://jq.qq.com/?_wv=1027&k=52j2FVO

相关文章
|
Cloud Native 算法 Go
解锁代码注释之谜:掌握代码注释的艺术与科学,提升软件开发与团队协作的卓越实践
解锁代码注释之谜:掌握代码注释的艺术与科学,提升软件开发与团队协作的卓越实践
132 1
|
24天前
|
敏捷开发 监控 数据可视化
团队协作新方法:任务故事墙如何优化项目进度管理?
任务总是卡壳、沟通不畅?试试任务故事墙!可视化管理让团队协作更流畅,高效解决项目瓶颈,快速提升效率!
41 2
团队协作新方法:任务故事墙如何优化项目进度管理?
|
2月前
|
测试技术 持续交付
掌握代码审查的艺术
《掌握代码审查的艺术》介绍了代码审查的重要性及其在提升软件质量和团队协作中的作用。文章涵盖代码审查的目的、准备、过程及频率等多个方面,并强调了使用自动化工具和持续学习的重要性。通过遵循这些技巧和最佳实践,团队不仅能发现潜在缺陷,还能促进知识共享和个人技能提升。最终,代码审查将成为推动团队协作与成长的强大工具。
|
2月前
|
安全 测试技术 开发者
软件测试的艺术:从代码审查到用户验收的全面探索
在软件开发的宇宙中,软件测试如同星辰指引,照亮了产品质量的道路。从单元测试的微观世界到集成测试的宏观视角,再到系统测试的广阔天地,以及最终的用户验收测试的终极考验,每一步都至关重要。本文将引领读者穿越这一系列测试阶段,揭示如何通过细致的规划、执行和评估,打造出无懈可击的软件产品。
|
3月前
|
缓存 监控 前端开发
前端代码评审问题总结年度代码翻车现场 |
团队已持续进行了一年多的线下周代码评审,作为主要评审人,我认识到虽然初衷是提供代码改进建议,但实际上大部分问题集中在基础代码质量上,而非设计或业务逻辑。因此,团队需保持耐心,逐步解决基础问题。本文总结了一年来常见的代码评审问题,如魔法值、eslint禁用、幽灵依赖等,并提出具体改进建议。此外,还强调了良好的代码习惯、命名规范及异常处理的重要性。通过持续代码评审,希望团队能在卓越工程的道路上不断进步。以下是常见问题的具体分析: ### 二、翻车现场(CR中的常见问题) #### 2.1 代码规范类 ##### 2.1.1 使用魔法值 - **危害**:代码不易读、不复用、易出错 - **建
36 3
|
7月前
|
缓存 前端开发 JavaScript
年度代码翻车现场 |前端代码评审问题总结
代码评审于技术团队的工程师文化建设非常有意义,它是形成团队统一代码风格最有效的方式,作者把自己团队在一年的CR中常见的那些小问题做了一些梳理,希望能对大家起到一点小帮助。
219832 7
|
7月前
|
算法 测试技术 持续交付
代码重构:提升软件质量的艺术与实践
代码重构:提升软件质量的艺术与实践
207 0
|
7月前
|
测试技术 项目管理
软件测试/测试管理|如何打造一份出彩的工作汇报!
软件测试/测试管理|如何打造一份出彩的工作汇报!
|
SQL 缓存 NoSQL
代码评审的18个军规,收藏好!
大家好,我是田螺。 我们开发完需求,提测前,一般都需要代码评审。小伙伴们,你们知道代码评审,一般都有哪些军规嘛?今天田螺哥给你带来代码评审的18个军规。
206 0
|
安全 Java 测试技术
关于代码评审(CodeReview)那些不得不说的事儿
关于代码评审(CodeReview)那些不得不说的事儿
262 1
关于代码评审(CodeReview)那些不得不说的事儿