[原]代码审查如何做

标签: | 发表时间:2017-01-22 23:02 | 作者:wzzfeitian
出处:http://blog.csdn.net/wzzfeitian

代码审查如何做


做 Code Review 的正反方观点

首先,我们先来看看正方的观点:

  1. Code review 中,可以通过大家的建议增进代码的质量。
  2. Code review 是一个传递知识的手段,可以让其它并不熟悉代码的人知道作者的意图和想法,从而可以在以后轻松维护代码。
  3. Code review 鼓励程序员们相互学习对方的长处和优点。
  4. Code review 可以被用来确认自己的设计和实现是清楚和简单的。

我们再来听下反方的声音:

很多开发Team抱怨Code Review就是一个形式,费时费力不说,发现的问题还不如测试,而评审者们除了在代码风格上有些见术,别的也就没什么用了,长而久之,大家都会开始厌烦这个事了。

那么为了这篇文章能够写下去, 笔者显然是属于正方了, 此处不再做争辩.


Review时, 什么该关注, 什么不该关注

在上面的Code Reivew中的正方观点中,我们没有提到可以帮助找到程序的bug和保证代码风格和编码标准, 那么是说Code Review不能帮助改善这几方面吗? 不是的, 这里确切地说, Code Review不是万能的, 在执行的时候, 应该专注于它应该关注的部分, 那么问题来了, Code Review时, 哪些是该关注的, 哪些又是不该关注的呢?

Code Review 时不该关注的

首先, 先列几个不该在Code Review时关注的

  1. Code review 不应该承担发现代码错误的职责。Code Review主要是审核代码的质量,如可读性,可维护性,以及程序的逻辑和对需求和设计的实现。代码中的bug和错误应该由单元测试,功能测试,性能测试,回归测试来保证的(其中主要是单元测试,因为那是最接近Bug,也是Bug没有扩散的地方)
  2. Code review 不应该成为保证代码风格和编码标准的手段。编码风格和代码规范都属于死的东西,每个程序员在把自己的代码提交团队Review的时候,代码就应该是符合规范的,这是默认值,属于每个人自己的事情,不应该交由团队来完成,否则只会浪费大家本来就不够的时间。

注意: 这里说在Code Review时不该关注, 并不是说不重要, 而是说, 这些部分应该有其他方式(例如自动化工具)去保障, 因为人的成本是最高的, 机器和程序才是便宜的.

Code Review 时该关注的

代码审查者在审查代码时有非常多的东西需要关注。一个团队需要明确对于自己的项目哪些点是重要的,并不断在审查中就这些点进行检查。

1. 体系结构和代码设计

  • 代码复用: 根据“三振法”, 如果代码被复制一次,虽然不喜欢这种方式,但通常没什么问题。但如果再一次被复制,就应该通过提取公共的部分来重构它。

  • 用更好的代码: 如果在一块混乱的代码做修改,添加几行代码也许更容易,但建议更进一步,用比原来更好的代码。

  • 潜在的bugs: 是否会引起的其他错误?循环是否以我们期望的方式终止?

  • 错误处理: 错误确定被优雅的修改?会导致其他错误?如果这样,修改是否有用?

  • 效率: 如果代码中包含算法,这种算法是否是高效? 例如,在字典中使用迭代,遍历一个期望的值,这是一种低效的方式。

  • 新代码与全局的架构是否保持一致?

  • 基础代码是否有结合使用了一些标准或设计样式,新的代码是否遵循当前的规范?代码是否正确迁移,或参照了因不规范而淘汰的旧代码?

  • 代码的位置是否正确?比如涉及订单的新代码是否在订单服务相关的位置?

  • 新代码是否重用了现存的代码?新代码是否可以被现有代码重用?新代码是否有重复代码?如果是的话,是否应该重构成一个更可被重用的模式,还是当前还可以接受?

  • 新代码是否被过度设计了?是否引入现在还不需要的重用设计?

2. 可读性和可维护性

  • 字段、变量、参数、方法、类的命名是否真实反映它们所代表的事物, 是否能够望文生义?

  • 是否可以通过读代码理解它做了什么?

  • 是否理解测试用例测了什么?

  • 测试是否很好地覆盖了用例的各种情况?它们是否覆盖了正常和异常用例?是否有忽略的情况?

  • 错误信息是否可被理解? log打的是否正确和足够?

  • 不清晰的代码是否被文档、注释或容易理解的测试用例所覆盖?具体可以根据团队自身的喜好决定使用哪种方式。

3. 功能

  • 代码是否真的达到了预期的目标?如果有自动化测试来确保代码的正确性,测试的代码是否真的可以验证代码达到了协定的需求?

  • 代码看上去是否包含不明显的bug,比如使用错误的变量进行检查,或误把and写成or?

  • 作者是否需要创建公共文档或修改现存的帮助文档?

  • 是否检查了面向用户的信息的正确性?

  • 是否有会在生产环境中导致应用停止运行的明显错误?代码是否会错误地指向测试数据库,是否存在应在真实服务中移除的硬编码的stub代码?

  • 你对性能的需求是什么,你是否考虑了安全问题?

    1. 这个新增或修复的功能是否会反向影响到现存的性能测试结果
    2. 外部调用很昂贵(a. 数据库调用. b. 不必要的远程调用. c. 移动或穿戴设备过频繁地调用后端程序)

4. 安全

  • 检查是否新的路径和服务需要认证
  • 数据是否需要加密
  • 密码是否被很好地控制?

    这里的密码包含密码(如用户密码、数据库密码或其他系统的密码)、秘钥、令牌等等。这些永远不应该存放在会提交到源码控制系统的代码或配置文件中,有其他方式管理这些密码,例如通过密码服务器(secret server)。当审查代码时,要确保这些密码不会悄悄进入你的版本控制系统中

  • 代码的运行是否应该被日志记录或监控?是否正确地使用?

    日志和监控需求因各个项目而不同,一些需要合规,一些拥有比别人严格的行为、事件日志规范。如果你有规章规定哪些需要记录日志,何时、如何记录,那么作为代码审查者你应该检查提交的代码是否满足要求。如果你没有固定的规章,那么就考虑:

    • 代码是否改变了数据(如增删改操作)?是否应该记录由谁何时改变了什么?
    • 代码是否涉及关键性能的部分?是否应该在性能监控系统中记录开始时间和结束时间?
    • 每条日志的日志等级是否恰当?一个好的经验法则是“ERROR”触发一个提示发送到某处,如果你不想这些消息在凌晨3点叫醒谁,那么就将之降级为“INFO”或“DEBUG”。当在循环中或一条数据可能产生多条输出的情况下,一般不需要将它们记录到生产日志文件中,它们更应该被放在“DEBUG”级别。

5. 其他方面

  • 是否合理地释放了资源

    1. 是否存在内存泄漏?
    2. 是否存在内存无限增长? 例如, 如果审查者看到不断有变量被追加到list或map中, 那么就要考虑下这个list或map什么时候失效, 或清除无用数据
    3. 代码是否及时关闭了连接或数据流?
    4. 资源池配置是否是否正确? 有没有过大或者过小?
  • 异常情况是否能够正确处理?

    1. 超时是否能够正确处理?
    2. 调用接口出错的时候, 是否有出错处理逻辑, 并且处理正确?
    3. 进程意外重启后, 是否能够恢复到崩溃前的环境?
  • 正确性(主要与多线程环境关系密切)

    1. 代码是否使用了正确的适合多线程的数据结构
    2. 代码是否存在竞态条件(race conditions)?多线程环境中代码非常容易造成不明显的竞态条件。作为审查者,可以查看不是原子操作的get和set
    3. 代码是否正确使用锁?和竞态条件相关,作为审查者你应该检查被审代码是否允许多个线程修改变量导致程序崩溃。代码可能需要同步、锁、原子变量来对代码块进行控制
    4. 代码的性能测试是否有价值?很容易将小型的性能测试代码写得很糟糕,或者使用不能代表生产环境数据的测试数据,这样只会得到错误的结果
    5. 缓存:虽然缓存是一种能防止过多高消耗请求的方式,但其本身也存在一些挑战。如果审查的代码使用了缓存,你应该关注一些常见的问题,如,不正确的缓存失效方式
  • 代码级优化, 对大部分并不是要构建低延时应用的机构来说,代码级优化往往是过早优化,所以首先要知道代码级优化是否必要

    1. 代码是否在不需要的地方使用同步或锁操作?如果代码始终运行在单线程中,锁往往是不必要的
    2. 代码是否可以使用原子变量替代锁或同步操作?
    3. 代码是否使用了不必要的线程安全的数据结构?比如是否可以使用ArrayList替代Vector?
    4. 代码是否在通用的操作中使用了低性能的数据结构?如在经常需要查找某个特定元素的地方使用链表
    5. 代码是否可以使用懒加载并从中获得性能提升?
    6. 条件判断语句或其他逻辑是否可以将最高效的求值语句放在前面来使其他语句短路?
    7. 代码是否存在许多字符串格式化?是否有方法可以使之更高效?
    8. 日志语句是否使用了字符串格式化?是否先使用条件判断语句校验了日志等级,或使用延迟求值?

Code Review 的实际操作建议

  1. 代码审查是应该在互相沟通中进行讨论的,而不是相互对抗。预先确定哪些是要点哪些不是,可以减少冲突并拟定预期。

  2. 经常进行Code Review, 不要攒了1w行才让同事帮你review, 这是坑队友.

    • 要Review的代码越多,那么要重构,重写的代码就会越多。而越不被程序作者接受的建议也会越多,唾沫口水战也会越多。
    • 程序员代码写得时候越长,程序员就会在代码中加入越来越多的个人的东西。 程序员最大的问题就是“自负”,无论什么时候,什么情况下,有太多的机会会让这种“自负”澎涨开来,并开始影响团队影响整个项目,以至于听不见别人的建议,从而让Code Review变成了口水战。
    • 越接近软件发布的最终期限,代码也就不能改得太多。
  3. Code Review不要太正式,而且要短

  4. 尽可能的让不同的人Reivew你的代码(不要超过3个人)

    • 从不同的方向评审代码总是好的。
    • 会有更多的人帮你在日后维护你的代码。
    • 这也是一个增加团队凝聚力的方法。
  5. 保持积极的正面的态度

    无论是代码作者,还是评审者,都需要一种积极向上的正面的态度,作者需要能够虚心接受别人的建议,因为别人的建议是为了让你做得更好;评审者也需要以一种积极的正面的态度向作者提意见,因为那是和你在一个战壕里的战友。记住,你不是一段代码,你是一个人!

  6. 学会享受Code Reivew


引用

  1. Code Review最佳实践
  2. Code Review中的几个提示
  3. 如何使代码审查更高效
  4. <<代码大全>>
作者:wzzfeitian 发表于2017/1/22 23:05:46 原文链接
阅读:3 评论:0 查看评论

相关 [代码审查] 推荐:

代码审查过程

- - 博客园_知识库
   英文链接: Code Review Processes.   对我而言,把代码产品化而没有合适的审查流程,就像是一场抽抽乐游戏. 代码当然也有可能会挺好,不过总还是有一定概率某人的哪块积木没抽好,然后一切就轰然崩塌. 无论是采用持续集成服务、结对审查、QA审查,还是所有这些方案的组合,都可以大大降低引入风险的概率.

Google 是如何做代码审查的

- litefy - python.cn(jobs, news)
在上一篇文章中提到过,我已经不在Google工作了. 我还没有想清楚应该去哪里—有两三个非常好的工作机会摆在我面前. 因为在这段做决定时间里,我不再受雇于任何人,我想可以写一些专业性的东西,一些很有趣,但也会在同事和管理工作中导致关系紧张的东西. Google是一个非常优秀的公司. 他们做出了很多令人称赞的东西—既是公司外部,人们可以看到的东西,也是公司内部.

代码审查中应该做的事

- China Moon - Solidot
威客 写道 "Mark Chu-Carroll从Google离职后,虽然已经收到了很多offer,但还没有决定去哪里. 他在其博客中分享了一些关于代码审查的趣事(中文). Google确实是一家很酷的公司. 不论是在公司内部或是外部,Google都做了很多让人赞叹的的事情. 这里我想介绍一些不涉及商业机密,但鲜为外人所知的事情.

敏捷代码审查指南

- - 博客 - 伯乐在线
“通过一次真正彻底地代码审查(code reviews),仔细阅读你的代码,找出问题,这是我知道的最好的方式去检测早期的bug,但是他们很少去这样干过. 某种意义上是因为他们花了大量的时间去写好代码,但是我认为主要是因为绝大部分 程序员害怕其他人审查自己的代码. 作为专业的程序员我们要克服阻力,如果你不愿意别人阅读你的代码,然后只是按照自己的意愿写,如果其他人没法读懂它,又怎能让别人使用呢.

代码审查(Code Review)清单

- - 博客 - 伯乐在线
代码审查可以帮助提高代码质量,避免由于代码习惯而造成的 bug. 下面列出的这些要点因该可以作为大部分代码审查的指导,如果是 Java 应用的话,这些建议应该被视作最佳实践. Javadoc 应该在每一个类和方法中添加. 如果是修复某个 bug,应该添加 bug ID. 走捷径的方法或者复杂的逻辑要有解释.

我所钟爱的代码审查

- - 译言-电脑/网络/数码科技
This is pretty standard fare for developers in the "real world", but I have never heard of an academic research group using them, and had never done code reviews myself before joining Google..

代码审查不是用来……

- - 外刊IT评论
提示:如果您在阅读器里点击订阅本站的文章链接时发现有一个中转页,这说明你的订阅地址有误,本站的订阅地址(RSS)是:. http://www.aqee.net/feed/,请及时纠正. 事实上,今天的我们正是从这种一直坚持探索的漫长道路上走出来的. 我们尝试各种技术、方法和工具,直到我们走到今天的成就(但这并不是说我们就此停步).

我们如何进行代码审查

- - CSDN博客研发管理推荐文章
本文来源于我在InfoQ中文站原创的文章,原文地址是:. Jim Bird是一位经验丰富的软件开发经理、项目经理与CTO,专注于软件开发与维护、软件质量与安全等领域中疑难问题的解决. 在过去的15年间,Jim曾管理过团队建设并主导过高性能的财务系统的建设. 他的主要兴趣在于如何提升小团队的效率以构建真正的软件:高质量、安全、可靠、高性能及适应性强.

你们公司做代码审查吗?

- - 外刊IT评论
每当从各种公司听到他们正在尝试自动化部署/测试的事情,我都非常关注,但通常会很吃惊,他们很少会考虑去实行代码审查制度. 看到这种情况,我通常想问: 如果代码没有经过其它人的审查,你如何知道你要测试的是什么. 这答案(如果有的话)通常是捏着手指头说 有几个人在做代码审查或“正在考虑中”. 不论你采用什么形式的测试过程,什么形式的部署过程,没有代码审查——game over.

[原]代码审查审什么

- - 阿朱=行业趋势+开发管理+架构
看着很多人做代码审查重点审格式和命名,制定的代码规范也主要偏重代码格式和命名,我真想骂一句操蛋,这真是浪费时间又解决不了问题. 此篇文章就是骂完操蛋后奋笔快速敲下来的,有不妥之处请大家谅解. 一、目的:为啥要花费时间要搞人工代码审查. 1、有些问题是工具检查不出来的,需要人工审查. 2、有些问题是不希望花大代价来发现、或者上线后才知道.