在代码审查中我们究竟想发现什么?- 开篇

Code Review是植根于工程师骨子里的一种态度

February 24, 2017 - 1 minute read -
web develop code

这是关于在代码审查中我们究竟想发现什么?系列6篇文章中的第1篇。

一直想找个时间节点把这个话题聊的具体点,因为Code Review事件在软件工程师的职场中发生的频率实在是太高了,同时也很大程度上决定了软件工程师早期的职业发展。如果你试着在网站找下这个话题,你可能会得到很多的歌颂Code Review好的文章,你能找到很多如何用工具来做Code Review,但是你可能很难找到一些质量很好的指导你如何去看他们Code的文章。可能的原因是不同的人在写代码时有各种各样的综合的因素要去考虑,要去迁就,比如功能方面需求的不同,或者是公司要求不同,等等。 Code Review是一个很大的课题,这篇文章的目的是概括性的描述出作为一个reviewer在做code review的过程中需要注意的方方面面。

当你去做Code Review的时候你一般都希望能看出哪些方面的问题呢?

不管你是用工具还是人工的去做code review,也不管在什么样的情况下,有些写code方面需要注意的问题总是很容易就区分出来,比如:

  • 格式:空格和换行在什么位置?有没有使用tab与空格?大括号是如何布局的?
  • 风格:变量或者参数是否被申明为final? 局部变量的被使用的位置是否靠近其定义的位置,或者局部变量是否定义在方法开始的位置?
  • 命名:field/constant/variable/param/class的名称是否符合标准?这些名称是否过短?
  • 测试覆盖:代码有没有测试? 这些都是有效的检查,你希望最小化不同的代码区域之间的上下文转换和减少认知负载,所以你的代码看上去越一致越好。 然而上面的这些code review的内容用人工去做审查并不十分合算,大量的这些方面的检查可以自动化。市面上现在有很多的这些方面的工具,它们可以保证你的代码有一致性的格式,标准化的命名,遵守final关键字的使用原则,并且可以发现一些由于简单的编码错误引起的bugs。比如你可以运行IntelliJ IDEA’s inspections from the command line,你不必依赖于所有的team members在他们的IDE里面有相同的审查。

什么是你应该要发现的?

设计

  • 新的代码如何融入已有的整体架构?
  • 代码是否遵循SOLID原则Domain Driven Design 以及团队喜欢的其他一些设计范式?
  • 在新的代码里面使用了哪些design patterns?使用的是否恰当?
  • 如有已经的基础代码有一些混合的标准后者设计风格,新的代码是否遵循了现有的一些实践?代码 是否在往正确的方向迁移,是否遵循老的要被淘汰的代码样例?
  • 代码是否写在正确的位置?比如,代码是与Orders相关,那么它是否被放在了Order Service中?
  • 新的代码是否复用了已有代码的东西?新的代码是否提供了一些可以被已有代码复用的东西?新代码是否引入了重复的内容?如果是,它们是否该被重构成更加可复用的模式,或者说在当前阶段,这样的代码是否是可接受的?
  • 代码是否是过分设计?为可复用性而构建的代码是否不再需要?团队该如何平衡考虑可复用性与YAGNI?

可读性与可维护性

  • 名称(field, variables, parameter, method and classed)真实的表达了所要表达的内容?
  • 你是否能够通过读代码来理解它们在做什么?
  • 测试是否有覆盖测试样例的子集?测试覆盖了happy paths和exceptional cases? 有没有没有想到的test cases?
  • 异常错误提示信息是否是可理解的?
  • 代码是否有一些容易引起误解的地方,如文档、评论或可理解的测试用例方面(根据团队的喜好)?

功能性

  • 代码确实做了它应该做的?是否有自动化测试来保证代码的正确性?测试确实满足已达成的测试要求?
  • 代码是否看上去包含一些小bugs,如使用了错误的变量名来做检查,后者意外的使用了and代替了or?

其他方面的考虑

  • 代码是否有潜在的安全问题?
  • 是否有政策层面的要求需要满足?
  • 哪些没有别自动化性能测试覆盖的区域,新的代码是否会引入不可以避免的性能问题,如一些不必要的database请求或者远程服务调用?
  • 作者是否需要创建公开的文档,或者修改已有的help文档?
  • 面向用户的信息是否做了正确性检查?
  • 是否存在会在产品中导致代码不能工作的明显错误?代码意外的指向测试环境的database,或者有一些在真实service中需要被清除而没有清理的硬编码stub?

每一个开发者都有自己的建议在如果写出“好”的代码这个话题上,在此只是抛砖引玉,希望有机会能跟大家一起探讨。尽请关照接下来关系这个话题的更细一级的讨论。

关于作者

Trisha Gee为一系列行业开发Java应用程序,包括金融、制造业、软件和非盈利组织,包括各种规模的公司。她在开发Java高性能系统上有丰富经验,并积极帮助开发者使他们拥有高生产率,她也涉足开源开发。Trisha是Sevilla Java用户组和Java Champion的带头人,她信任社区并分享自己的观点,帮助我们从错误中学习,并在成功的基础上继续发展。她是JetBrains技术推广人,也就是说她会不断分享所有有趣的发现。

原文来自UPSOURCE BLOG, 谢谢!👏🏻