【第944期】代码审查应该关注什么之一
前言
是否经常听过review代码这个事情,那到底要review哪些呢?且看今日早读文章,由@唐先僧翻译分享。
正文从这开始~
本文只是其一
我们一起来讨论下代码审查。如果你花几秒钟时间搜索一下代码审查的信息,你会发现很多文章都在讲为什么代码审查是件好事(比如Atwood 的这篇博客)。
你也会发现很多关于如何使用代码审查工具的文档(比如我们自己的 Upsource)。
然而你很少会发现一些指南,会告诉你作为代码审查者在审查别人代码时需要关注哪些东西。
之所以没有权威的文章告诉你在代码审查中需要关注哪些东西,其原因可能是:有太多不同的事情需要关注。并且,像其他的需求(功能性的或者非功能性的)一样,不同的团队对每个方面有不同的优先级。
因为这是一个很大的主题,本文的目的仅仅是列出大纲,用于说明作为代码审查者在审查代码时需要关注的东西。决定各个方面的优先级并不断的检验也是一个非常复杂的主题,本身就可以单独写出一篇文章。
当你在审查其他人的代码时,你在关注什么?
不管你是通过像 Upsource 这样的工具还是同事的讲解来审查代码,有一些东西是比较容易评判的。比如:
格式:空格和换行在哪里?他们使用的是 tab 还是空格?大括号怎么布局?
风格:变量/参数声明为 final ?方法变量是在使用时再定义,还是在方法开始的地方定义?
命名:字段、常量、变量、参数、类的命名遵循标准了吗?名称有过于简短吗?
代码覆盖率:这段代码有写测试代码吗?
检查这些东西都是有意义的--你希望把不同代码之间的切换最小化并且减少认知负担,所以你的代码看起来越一致越好。
然而,在你的团队中,使用人力检查这些也许不是对时间和资源的最佳利用,因为很多这样的检查都可以自动化进行。有很多工具可以保证:你的代码格式连贯一致,遵循命名标准和使用 final 关键字,并且可以发现一些简单的编程错误导致的 bug。例如,你可以通过命令行使用 IntelliJ IDEA 的检查,所以你不必要求所有的团队成员都在他们的 IDE 中运行检查。
你应该关注什么?
人类真正擅长的是哪几类事情?什么是东西是我们在代码审查中发现但是检查工具发现不了的?
事实证明有很多事情。这当然也不是一个详尽的清单,我们也不会在这里就某一项进行详细讨论。然而,你的团队应该就代码审查应关注什么,展开交流,并且也许是你应该关注的。
设计
新的代码怎么符合总体的架构?
代码遵循 SOLID原则,领域驱动设计或者其他你的团队喜欢的设计风格吗?
新的代码中使用了什么设计模式?这些设计模式合适吗?
如果代码库有多种标准或者设计风格,新的代码和当前流行的一致吗?代码是向正确的方向转移,还是遵循将被淘汰的旧代码?
代码是处在正确的位置?例如,如果代码是和订单相关的,它们是否处于 Order Services?
新的代码有复用现有的代码中的一些东西吗?新的代码有提供一些现有代码可以复用的东西吗?新的代码有没有引入重复?如果有,应该重构成更加可复用的风格,还是这个阶段也可以接受?
代码是否过度工程化?代码有没有创建现在并不需要的重用性?你的团队怎么根据 YAGNI平衡考虑重用性?
可读性和可维护性
字段、变量、参数、方法以及类的名称是否真实反映它们所代表的事物?
我通过阅读代码能够理解代码在做什么事情吗?
我能理解测试代码在做什么吗?
测试用例覆盖了好的用例?测试用例覆盖了正常场景和异常场景吗?有没有还没考虑到的场景?
异常情况的错误消息好理解吗?
令人困惑的代码段有没有文档描述、或者代码注释或者通过容易理解的测试用例覆盖(根据团队喜好)?
功能性
代码是在做预期的事情吗?如果有自动测试来保证代码的正确性,测试代码真的是按照商定的需求来测试的吗?
代码看起来包含隐藏的 bug 吗?比如使用了错误的变量检查,或者偶然使用 逻辑与 取代了 逻辑或。
你有考虑过。。。?
是否存在潜在的安全问题?
有没有监管的需求要满足?
对于自动化性能测试没有覆盖的区域,新的代码是否引入了可以避免的性能问题,比如不必要的数据库访问或者远程服务访问?
作者需要创建公共文档吗,或者需要修改已有的帮助文件吗?
用户交互信息有做过正确性检查吗?
是否存在明显的错误将导致生产终止?代码是否会偶然指向测试数据库,或者是否存在硬编码需要在真实服务中移除?
最后,各项可持续关注。@唐先僧曾分享过:
你们团队有做code Review的习惯吗?有的话,一般都怎么做呢?
关于本文
译者:@唐先僧
译文:http://www.jianshu.com/p/a0dffa53e4b8
作者:@Trisha Gee
原文:https://blog.jetbrains.com/upsource/2015/07/23/what-to-look-for-in-a-code-review/