代码复审

一、代码复审的目的

  1、找出代码的错误。如：
        a. 编码错误，比如一些能碰巧骗过编译器的错误。
        b. 不符合项目组的代码规范的地方。
  2、发现逻辑错误，程序可以编译通过，但是代码的逻辑是错的。
  3、发现算法错误，比如使用的算法不够优化。
  4、发现潜在的错误和回归性错误——当前的修改导致以前修复的缺陷又重新出现。
  5、发现可能改进的地方。
  6、教育（互相教育）开发人员，传授经验，让更多的成员熟悉项目各部分的代码，同时熟悉和应用领域相关的实际知识。

二、代码复审的步骤

  1  、在复审前代码必须成功地编译，在所有要求的平台上，同时要编译DeBug|Retail 版本。编译要用团队规定的最严格的编译警告等级 （例如C/C++中的W4  ）。
  2  、 程序员必须测试过代码。什么叫测试过？最好的方法是在DeBugger 中单步执行。
  3  、程序员必须提供新的代码，以及文件差异分析工具。Windiff 或VSTS 自带的工具都可以。VSTS 中可以通过Shelveset 来支持远程代码复审。
  4  、复审者可以选择面对面的复审、独立复审或其他方式。
  5  、在面对面的复审中，一般是开发者控制流程，讲述修改的前因后果。但是复审者有权在任何时候打断叙述，提出自己的意见。
  6  、复审者必须把反馈意见逐一提出。注意，复审者有权提出很多看似吹毛求疵的问题，复审者不必每一件事都要亲自调查，开发者有义务给出详尽的回答。
  7  、开发者必须负责让所有的问题都得到满意的解释或解答，或者在TFS 中创建新的工作项以确保这些问题将来会得到处理。
  8  、对于复审的结果，双方必须达成一致的意见。
        a. 打回去——复审发现致命问题，这些问题解决之前不能签入代码；
        b. 有条件地同意——发现了一些小问题，在这些问题得到解决或记录之后，代码可以签入，不需要再次复审；
        c. 放行——代码可以不加新的改动，签入源码控制服务器。

        要记住复审者是通过问这些问题来确保软件质量的，而不是有意找碴。
       避免不必要的繁文缛节，我们做代码复审的目的是为了减少错误的发生，而不是找一个人来对着你的代码点头。一些简单的修改不是非得要一个复审者来走一遍形式。在项目开发的早期斤斤计较于一些细枝末节  例如：帮助文件里的拼写错误，数据文件格式不够最优化等  、也是于大局无补的，但是，这些问题并不是不用处理了，我们必须建立一些优先级较低的工作项来跟踪这些事情。

三、在代码复审中还要做什么

       好的复审者不光是要注意到程序员修改了什么，还要把眼光放远，问一些这样的问题：

           “你这样修改了之后，有没有别的功能会受影响?”
           “项目中还有别的地方需要类似的修改么?”
           “有没有留下足够的说明，让将来维护代码时不会出现问题？”
           “对于这样的修改，有没有别的成员需要告知?”
           “导致问题的根本原因是什么？我们以后如何能自动避免这样的情况再次出现？”

     有些修改看似聪明有效率，但是这样的修改有可能会让以后的开发和维护更困难。

四、在代码复审后要做什么

       人不能两次踏入同一条河流，程序员不能两次犯同样的错误。在代码复审后，开发者应该把复审过程中的记录整理出来：
    （1）更正明显的错误。
    （2）对于无法很快更正的错误，要在TFS中创建Bug把它们记录下来。
    （3）把所有的错误记在自己的一个“我常犯的错误”表中，作为以后自我复审的第一步。
      有些人喜欢在程序中加一些特定的标记，来跟踪各种“要做的事情”，例如：

      //$todo:  make this function thread-safe      //$review: is this function thread-safe?  Need to look at it again      //$bug: when input array is very large, this function might lead //to crash

      这些标记最好是加上人名，以示负责。如：

      //$bug (AChao): when input array is very large, this function will //   become very slow due to O(N*N) algorithm。

        在代码复审过程中，$review标记的问题要一一讨论，在代码复审过后，所有的$review标记要清除。在一个里程碑或正式版本发布之前，所有的$todo:$bug:标记都要清除。

       做标记是不错的办法，但是如果开发者光记得做标记，最后却没有真正去研究和改正这些潜在的问题，这些todo、review、Bug就会被遗弃在代码中。过了一段时间后，后来的程序员也不敢碰它们——因为没有人能真正了解上一个版本的$todo是真的要马上做，还是已经做过了（done）了，只是没有更新$todo的注释，或者问题早已通过别的方式解决了。其根本原因在于团队没有用TFS（或者其他的管理软件）进行记录，没有人会跟踪这些事情。

五、代码复审的核查表

  1．概要部分
     （1）代码能符合需求和规格说明么？
     （2）代码设计是否有周全的考虑？
     （3）代码可读性如何？
     （4）代码容易维护么？
     （5）代码的每一行都执行并检查过了吗？
  2．设计规范部分
     （1）设计是否遵从已知的设计模式或项目中常用的模式？
     （2）有没有硬编码或字符串/数字等存在？
     （3）代码有没有依赖于某一平台，是否会影响将来的移植（如Win32到Win64）？
     （4）开发者新写的代码能否用已有的Library/SDK/Framework 中的功能实现？在本项目中是否存在类似的功能可以调用而不用全部重新实现？
     （5）有没有无用的代码可以清除？（很多人想保留尽可能多的代码，因为以后可能会用上，这样导致程序文件中有很多注释掉的代码，这些代码都可以删除，因为源代码控制已经保存了原来的老代码。）
  3．代码规范部分
       修改的部分符合代码标准和风格么（详细条文略）?
  4．具体代码部分
     （1）有没有对错误进行处理？对于调用的外部函数，是否检查了返回值或处理了异常？
     （2）参数传递有无错误，字符串的长度是字节的长度还是字符（可能是单/双字节）的长度，是以0 开始计数还是以1 开始计数？
     （3）边界条件是如何处理的？Switch 语句的Default 是如何处理的？循环有没有可能出现死循环？
     （4）有没有使用断言（Assert）来保证我们认为不变的条件真的满足？
     （5）对资源的利用，是在哪里申请，在哪里释放的？有没有可能导致资源泄露（内存、文件、各种GUI 资源、数据库访问的连接，等等）？有没有可能优化？
     （6）数据结构中是否有无用的元素？
  5．效能
     （1）代码的效能（Performance）如何？最坏的情况是怎样的？
     （2）代码中，特别是循环中是否有明显可优化的部分（C++中反复创建类，C#中 string 的操作是否能用StringBuilder 来优化）？
     （3）对于系统和网络调用是否会超时？如何处理?
  6．可读性
       代码可读性如何？有没有足够的注释？
  7．可测试性
       代码是否需要更新或创建新的单元测试？