代码审查应该关注什么

本文是“代码审查关注什么”系列文章的第一篇(共六篇)。

我们一起来讨论下代码审查。如果你花几秒钟时间搜索一下代码审查的信息,你会发现很多文章都在讲为什么代码审查是件好事(比如Atwood 的这篇博客)。

你也会发现很多关于如何使用代码审查工具的文档(比如我们自己的 Upsource)。

然而你很少会发现一些指南,会告诉你作为代码审查者在审查别人代码时需要关注哪些东西。

之所以没有权威的文章告诉你在代码审查中需要关注哪些东西,其原因可能是:有太多不同的事情需要关注。并且,像其他的需求(功能性的或者非功能性的)一样,不同的团队对每个方面有不同的优先级。

因为这是一个很大的主题,本文的目的仅仅是列出大纲,用于说明作为代码审查者在审查代码时需要关注的东西。决定各个方面的优先级并不断的检验也是一个非常复杂的主题,本身就可以单独写出一篇文章。

当你在审查其他人的代码时,你在关注什么?

不管你是通过像 Upsource 这样的工具还是同事的讲解来审查代码,有一些东西是比较容易评判的。比如:

  • 格式:空格和换行在哪里?他们使用的是 tab 还是空格?大括号怎么布局?
  • 风格:变量/参数声明为 final ?方法变量是在使用时再定义,还是在方法开始的地方定义?
  • 命名:字段、常量、变量、参数、类的命名遵循标准了吗?名称有过于简短吗?
  • 代码覆盖率:这段代码有写测试代码吗?

检查这些东西都是有意义的--你希望把不同代码之间的切换最小化并且减少认知负担,所以你的代码看起来越一致越好。

然而,在你的团队中,使用人力检查这些也许不是对时间和资源的最佳利用,因为很多这样的检查都可以自动化进行。有很多工具可以保证:你的代码格式连贯一致,遵循命名标准和使用 final 关键字,并且可以发现一些简单的编程错误导致的 bug。例如,你可以通过命令行使用 IntelliJ IDEA 的检查,所以你不必要求所有的团队成员都在他们的 IDE 中运行检查。

你应该关注什么?

人类真正擅长的是哪几类事情?什么是东西是我们在代码审查中发现但是检查工具发现不了的?

事实证明有很多事情。这当然也不是一个详尽的清单,我们也不会在这里就某一项进行详细讨论。然而,你的团队应该就代码审查应关注什么,展开交流,并且也许是你应该关注的。

设计

  • 新的代码怎么符合总体的架构?
  • 代码遵循 SOLID原则,领域驱动设计或者其他你的团队喜欢的设计风格吗?
  • 新的代码中使用了什么设计模式?这些设计模式合适吗?
  • 如果代码库有多种标准或者设计风格,新的代码和当前流行的一致吗?代码是向正确的方向转移,还是遵循将被淘汰的旧代码?
  • 代码是处在正确的位置?例如,如果代码是和订单相关的,它们是否处于 Order Services?
  • 新的代码有复用现有的代码中的一些东西吗?新的代码有提供一些现有代码可以复用的东西吗?新的代码有没有引入重复?如果有,应该重构成更加可复用的风格,还是这个阶段也可以接受?
  • 代码是否过度工程化?代码有没有创建现在并不需要的重用性?你的团队怎么根据 YAGNI平衡考虑重用性?

可读性和可维护性

  • 字段、变量、参数、方法以及类的名称是否真实反映它们所代表的事物?
  • 我通过阅读代码能够理解代码在做什么事情吗?
  • 我能理解测试代码在做什么吗?
  • 测试用例覆盖了好的用例?测试用例覆盖了正常场景和异常场景吗?有没有还没考虑到的场景?
  • 异常情况的错误消息好理解吗?
  • 令人困惑的代码段有没有文档描述、或者代码注释或者通过容易理解的测试用例覆盖(根据团队喜好)?

功能性

  • 代码是在做预期的事情吗?如果有自动测试来保证代码的正确性,测试代码真的是按照商定的需求来测试的吗?
  • 代码看起来包含隐藏的 bug 吗?比如使用了错误的变量检查,或者偶然使用 逻辑与 取代了 逻辑或

你有考虑过。。。?

  • 是否存在潜在的安全问题?
  • 有没有监管的需求要满足?
  • 对于自动化性能测试没有覆盖的区域,新的代码是否引入了可以避免的性能问题,比如不必要的数据库访问或者远程服务访问?
  • 作者需要创建公共文档吗,或者需要修改已有的帮助文件吗?
  • 用户交互信息有做过正确性检查吗?
  • 是否存在明显的错误将导致生产终止?代码是否会偶然指向测试数据库,或者是否存在硬编码需要在真实服务中移除?

敬请期待后续文章,我们将详细讨论这些主题。

本文译自What to look for in a Code Review

最后编辑于
©著作权归作者所有,转载或内容合作请联系作者
  • 序言:七十年代末,一起剥皮案震惊了整个滨河市,随后出现的几起案子,更是在滨河造成了极大的恐慌,老刑警刘岩,带你破解...
    沈念sama阅读 199,271评论 5 466
  • 序言:滨河连续发生了三起死亡事件,死亡现场离奇诡异,居然都是意外死亡,警方通过查阅死者的电脑和手机,发现死者居然都...
    沈念sama阅读 83,725评论 2 376
  • 文/潘晓璐 我一进店门,熙熙楼的掌柜王于贵愁眉苦脸地迎上来,“玉大人,你说我怎么就摊上这事。” “怎么了?”我有些...
    开封第一讲书人阅读 146,252评论 0 328
  • 文/不坏的土叔 我叫张陵,是天一观的道长。 经常有香客问我,道长,这世上最难降的妖魔是什么? 我笑而不...
    开封第一讲书人阅读 53,634评论 1 270
  • 正文 为了忘掉前任,我火速办了婚礼,结果婚礼上,老公的妹妹穿的比我还像新娘。我一直安慰自己,他们只是感情好,可当我...
    茶点故事阅读 62,549评论 5 359
  • 文/花漫 我一把揭开白布。 她就那样静静地躺着,像睡着了一般。 火红的嫁衣衬着肌肤如雪。 梳的纹丝不乱的头发上,一...
    开封第一讲书人阅读 47,985评论 1 275
  • 那天,我揣着相机与录音,去河边找鬼。 笑死,一个胖子当着我的面吹牛,可吹牛的内容都是我干的。 我是一名探鬼主播,决...
    沈念sama阅读 37,471评论 3 390
  • 文/苍兰香墨 我猛地睁开眼,长吁一口气:“原来是场噩梦啊……” “哼!你这毒妇竟也来了?” 一声冷哼从身侧响起,我...
    开封第一讲书人阅读 36,128评论 0 254
  • 序言:老挝万荣一对情侣失踪,失踪者是张志新(化名)和其女友刘颖,没想到半个月后,有当地人在树林里发现了一具尸体,经...
    沈念sama阅读 40,257评论 1 294
  • 正文 独居荒郊野岭守林人离奇死亡,尸身上长有42处带血的脓包…… 初始之章·张勋 以下内容为张勋视角 年9月15日...
    茶点故事阅读 35,233评论 2 317
  • 正文 我和宋清朗相恋三年,在试婚纱的时候发现自己被绿了。 大学时的朋友给我发了我未婚夫和他白月光在一起吃饭的照片。...
    茶点故事阅读 37,235评论 1 328
  • 序言:一个原本活蹦乱跳的男人离奇死亡,死状恐怖,灵堂内的尸体忽然破棺而出,到底是诈尸还是另有隐情,我是刑警宁泽,带...
    沈念sama阅读 32,940评论 3 316
  • 正文 年R本政府宣布,位于F岛的核电站,受9级特大地震影响,放射性物质发生泄漏。R本人自食恶果不足惜,却给世界环境...
    茶点故事阅读 38,528评论 3 302
  • 文/蒙蒙 一、第九天 我趴在偏房一处隐蔽的房顶上张望。 院中可真热闹,春花似锦、人声如沸。这庄子的主人今日做“春日...
    开封第一讲书人阅读 29,623评论 0 19
  • 文/苍兰香墨 我抬头看了看天上的太阳。三九已至,却和暖如春,着一层夹袄步出监牢的瞬间,已是汗流浃背。 一阵脚步声响...
    开封第一讲书人阅读 30,858评论 1 255
  • 我被黑心中介骗来泰国打工, 没想到刚下飞机就差点儿被人妖公主榨干…… 1. 我叫王不留,地道东北人。 一个月前我还...
    沈念sama阅读 42,245评论 2 344
  • 正文 我出身青楼,却偏偏与公主长得像,于是被迫代替她去往敌国和亲。 传闻我的和亲对象是个残疾皇子,可洞房花烛夜当晚...
    茶点故事阅读 41,790评论 2 339

推荐阅读更多精彩内容