【转载】代码评审指导

转载地址;http://www.oschina.net/translate/code-review-guidelines

代码评审就是源代码的系统性审核（也叫同业互查），目的是找到并且修复在开发最初阶段被忽视掉的一些问题，以此来改进软件质量，同时提高程序员的编码能力。
代码评审为什么重要呢？
引自“Code Complete”
“软件测试本身有一定的局限性，在检测软件缺陷中，单元测试发现缺陷的比例大概是25%，功能测试占到35%，集成测试占到45%。相比之下，设计和代码审查的效率要高很多，发现缺陷的比例可以达到55%-60%。对审查结果的案例学习也是很给力的。
以下是一些数据分析来说明通过代码审查可以及早发现软件问题从而降低成本的重要性。


代码审查的主要目标有：
1.尽早的发现和修复代码中的缺陷。
2.以更好的共享和理解代码作为团队成员相互学习的基础。
3.有助于保持设计和实现的一致性。
4.帮助发现大家容易犯的共同错误，从而减少团队的返工。
5.构建投资者对执行过程中技术质量的信心。
6.大家对代码有一个平均的理解，这有助于提高团队成员的互换性，特别当某个成员无法继续工作的时候就显得尤为重要。
7.从不同的角度看问题。“另外一双眼睛”增加了客观性。这个原因也是把开发和测试团队分开的原因，同行评审代码发现问题需要提供给他们一定的距离（译者注：这个距离可以使他们和代码的作者从不同的角度看问题）。
8.骄傲/奖励。对他编码能力的认可对许多程序员来说，是一个显著的奖励。
9.团队凝聚力。一起工作可以让团队成员变得更加紧密。它还可以让因为写代码而造成的隔离得到一个短暂的释放。


代码审查者所关注的地方主要有以下几点：
通用的单元测试
注释和编码规范
错误处理
资源泄漏
线程安全
控制结构
性能
功能
安全
角色和职责
1.开发者：是要审查的代码的作者，并且也是发起审查活动的人。
2.审查者：是审查代码并且要报告审查结果给开发者的人。


像任何技能，再完美的评审工作都来源于实践。如下是一些提示，可能对您选择正确的途径有帮助：
对开发人员的提示：
1.最初的评审者应该是作者自己。
2.为自己的评审的代码更趋于关注的东西创建清单。这些清单中有些是很容易收集整理的。它应该遵循编码标准文档的大纲，因为这是你的评审清单，即便是有问题，你可以抓住你很难做的东西，并且跳过你认为很简单的东西。按照你列出来的清单往下测试你的代码，并修改你发现的问题。这样你不仅仅是减少了团队其他人发现的问题，而且也减少了评审会议的时间，大家都会非常高兴能够使用很短的时间开评审会议。
3.你和你的代码不等同。记得整个评审的目的是为了发现问题，并且问题是一定会被发现的。有人发现了你的问题，千万不要介意啊。
4.要理解并接受你所犯的错误。评审的目的是尽早的发现问题，避免将问题带到以后的产品中去。幸运的是，除了在JPL我们几个开发rocket guidance software，我们的行业很少有致命的错误，所以我们可以，我们应该学会笑，然后继续。
5.无论你知道的"karate"有多么的多，其他人也总是会知道的更多。如果你问，这样的人员可以教你一些新举措。寻求和接受别人的意见，特别是当你觉得不需要的时候。
6.未经咨询讨论，不重写代码。”重写代码“和”修改代码“只是一步之遥，知道差异、追求在代码审查的框架内的风格变化，不要一个人孤独的奋战。
7.世界上唯一不变的就是变化。敞开心扉，并微笑着接受它。留心对您的需求、平台或工具都作为新的挑战的每一个变化，不至于最后严重到不可收拾的地步。
8.为你的信念而战，但从容地接受失败。要明白，有时候你的想法会被推翻，即使你被证明是正确的，不被报复或者数落，"我告诉过你了"这句话至少在好几次以上，并且不让你离开的想法滋生。
9.不要做"房间里的人"。不要成为在黑暗处编码只为买可乐的人。房间里的人和外界失去了联系，看不见外面，并且失控，他没有开放、协作的工作环境。
10.请注意审查会议不是解决问题的会议。
11.有助于维护编码标准。提供要添加到编码标准中的东西，这些事我们讨论时编码标准中没有的东西。挑战之一，开发人员已经组织比较麻烦的代码审查工作，他们往往不知道接下来的问题将从哪里来。如果你把每个问题的文档编写标准，下次你的代码审查你可以用清单检查它。它还会帮到你巩固你已经掌握的概念，让您更不容易错过反馈的机会。


给审查者的提示:
1. 批评代码而不是人。体贴开发者而不是代码。尽可能的使用正面、积极的，并且有利于提高代码质量的评价。评价要与项目标准、开发守则、性能提升等因素相关。
2. 尊重了解程度不如你的人，并保持耐心。非技术人员在面对开发人员时，通常认为恃才傲物的比较牛逼，自卑的比较烂。不要用愤怒与焦躁来增强这样的陈规旧习。
3. 真正的权威来源于学识，而不是地位。学识造就权威、权威带来尊重。
4. 注意！审核代码的会议并不意味着解决问题的会议。
5. 使用疑问句而不是肯定句。肯定句是刺耳的。“在这里，你并没有遵循标准”，这样的话语就是一种攻击，无论你是否有意而为之。“是什么原因促使你使用现在的方法？”会得到更多的信息。很显然，这样的问题无法以一种讽刺或是傲慢的语气来表述；但这样做是正确的，如此的对话会让开发人员开始思考，继而寻找更好的方法。
6. 避免使用“为什么”。尽管很难避免，但如此而为能够充分地改善气氛。正如肯定句刺耳一样，“为什么”也如同一把尖刀。大多数的“为什么”能够经过变过而被省略，并达到激动人心的效果。例如，“你为什么没有遵循标准？”->”出于何处原因在这里对标准做了一些变化？”
7. 记得要赞美。审核代码的目的不仅仅是告诉开发人员如果提高，同样也要告诉他们做得好。人类的本性就是想要被认可，不要仅仅体现出我们的过错。因为开发是一项创造性的工作，开发人员用”心“在写代码，所以这更需要赞美，即使更多的批判。
8. 确保你有良好的编码规范作为参考。审核者遵循的依据应当是组织的编码规范。编码规范是一份被开发人员普遍认可的协议，用于编写优质的代码。如果要讨论一项并不在代码规范中的代码，你首先要做的工作是建立相关的代码规范。你应该不断地询问自己是否在讨论编码规范中的事项。
9. 条条大路通罗马。尽管开发者使用的方法与你想象的大相径庭，但不一定是错的。审核代码的目标是代码质量。如果达到了目标并遵循了标准，这就是你想要的。
10. 不要草草地进行代码审核，但需要及时的。你的同伴正在等你审核！
11. 一次审核200-400行代码。

安全代码审查
如果一个程序需要严密关注安全，那么 www.owasp.org 是一个获取资源的好地方。这里有一个非常好的安全代码审查文档可以参照： https://www.owasp.org/images/2/2e/OWASP_Code_Review_Guide-V1_1.pdf
代码审查结果的指定严重程度
代码中发现的问题的严重程度应该如下所示。审查者必须首先聚焦高级严重程度的问题，然后是中级严重程度的问题，最后是低级严重程度的问题。
命名规则和代码风格=低级
控制结构或逻辑问题=中级
冗余问题=高级
性能问题=高级
安全问题=高级
可测量性问题=高级
功能性问题=高级
错误处理=高级
可重用性问题=中级


开发者和审查者的检查表
对于代码审查来说，检查表是一个非常推荐的方式，它可以让你很容易的找到你忘记的那些事情，并且对不管是作者还是审查者来说都十分有用。遗漏是最难发现的缺陷，毕竟要去审查那些不存在的东西是很难发现的。检查表是最好的解决这个问题的办法，它可以提醒作者或审查者去花时间寻找是否遗漏了一些东西。检查表也可以让作者和审查者确认：所有的错误都将会被处理，所有的函数的参数都可以经受不正确的值测试，单元测试已经被建立起来。
另外一个实用的原则是个人的检查表。每个人通常都会犯同样的15-20个错误。如果你注意到你通常所犯的错误是哪些，你可以做一个你自己的检查表（PSP，SEI，CMMI也推荐这么做）。审查者通常检查你所犯的一般错误。所有你需要去做的是保持一个小的检查表列出你工作中的一般错误，特别是那些你忘记去做的事情而导致的错误。一旦你开始在一个审查表中记录你所犯的错误，你就必须开始让这个检查表逐渐变的小起来。这个规则将在你的头脑里不断的闪现，你的错误率会下降。我们已经看到了这种现象的反复发生。


为程序员准备的检查单
检查项
是否符合?
代码没有编译错误
代码不仅包含单元测试用例，而且测试均已通过。
代码包含了恰当的注释和代码文档（如：JavaDoc）
代码是清晰明了的，包括缩进、行长、没有保留注释的代码、没有单词拼写错误，诸如此类)
恰当的使用了异常
恰当的使用了Log，方便运维的定位错误。
删除了未用的import语句
消除了所有 Eclipse 报的警告错误
已尽可能的考虑了NPE
所有代码已符合代码规范
代码中没有遗留模板代码和测试用代码
代码中没有使用硬编码，也没有仅用于开发环境的代码。
性能问题是否有考虑？
安全问题是否有考虑？
代码中是否适当的释放了资源，比如HTTP连接、数据库连接、文件句柄等等。
框架中隐晦部分或特殊情况以文档化或者其它等价的方式说明。
是否优先使用可重用组件或库中相同功能的函数来替换自己的实现？
确保线程完全，避免死锁。
为评审人员准备的检查单
检查项
是否符合?
评审意见要通俗易懂并侧重于代码的可维护性。
评审意见是否言简意赅。
尽可能的使用泛型。
实参被恰当的使用。
异常被恰当的使用。
重复代码已被剔除。
框架被恰当的使用 – 方法被恰当的定义。
命令类被设计成只对应于某个单独的功能，而不是多合一。
JSPs代码里不能包含业务代码
代码中附带正确的单元测试用例
常见问题被标出。
潜在的线程问题被尽可能的排除。
所有考虑到的安全问题都被解决。
代码考虑了性能问题
代码实现与当前的产品设计及技术架构保持一致。
代码是可测试的
代码中是否包含不必要的静态方法或代码段。
代码完全符合代码标准
恰当的使用了Log，包括正确的Log级别和Log描述。
检查NPE和AIOB