【51CTO.com快译】在今天的文章中，我们将探讨如何审查他人编写的代码。这意味着我们需要将立场转换为审查者，并期望代码提交者能够遵循理想的开发原则及最佳实践，从而减轻我们的工作强度。
大家可以采取多种方式完成代码审查任务，这里我们将列出一份分布清单，帮助大家有效缩短审查时间并提升审查效率。
免责声明
每段代码、每个项目乃至每支团队都拥有自己的特性，因此本文只作为指导性参考，而非必须严格执行的硬性规则。
另外，请注意代码审查中几乎必然涵盖代码阅读与审查问题沟通等因素。因此，这里我们主要着眼于提升沟通效果，而非更加具体的代码内容。大家需要根据实际情况判断具体代码内容是否符合要求。
1.由参与度最高的人负责回答问题
假定您本人并没有参与代码编写(而单纯负责代码审查)，那么大家首先需要意识到，您并不清楚代码编写中的背景或者具体条件。因此，不要轻易将你不太理解部分视为错误，而要知道每个人思考问题的思路都不一样——也就是给他人解释的机会。
以此为基础，为以下问题找到答案：
如果确实存在问题，了解为何会出现这类问题。和代码编写者沟通你所发现的问题会带来哪些严重的影响时，如果你有解决方案，请将其作为建议提出。如果您没有解决办法，但可以就解决问题给出指导，请确保沟通到位。如果您认为某种特定设计模式能够解决问题，请详尽告知。如果您认为代码中的其它功能或者部分可以用于解决此问题，应明确指出。无论您指出怎样的解决方案，只要不偏离主题，相信都能为代码编写者带来帮助。
如果不属于问题，不要将其作为问题指出。如果被代码编写者误以为你表述内容是代码有问题，则其会对其进行修改，而这样的修改往往并未经过具体讨论。颇为讽刺的是，即使对代码内容的修改幅度不大，但也会引发新的问题。更为糟糕的是，您的建议可能由于不够明确或者存在歧义而使得开发者对代码内容进行多次迭代。总之，坦诚表达自己的看法能让真正的问题得到重视与改正，要注意哪些观点仅仅出于有疑惑而非真正属于错误。
让问题成为问题。不要自行寻找答案，而应由开发者给出直接解释。具体来讲，不要对问题表述进行过度修辞性修饰，也不要引导对方给出您潜意识里想要的答案。
让对方自己做调查。开发者对于代码更为熟悉，因此其对代码的编写及内容也更有发言权。他们能够比您更快地找到更为确切的答案。然而，您亲自进行调查可能同样会带来价值，前提是您愿意在审查周期之外拿出时间考虑代码的内部运作机制。
以下是大家经常会范的错误：
修改前：
1. 假定某些代码一定表现糟糕。表达内容不清，未明确提出问题所在，而假定受审查方未能够猜到您想传达的观点。
2. 为什么执行效果不佳?是不是应该进行修改?
3. 性能表现是否存在问题?其会造成哪些影响?(具体描述。)
修改后：
我认为这条SQL查询能够给出正确的结果集，但我觉得其可能执行了表扫描或者并未充分使用索引机制，不过我对此并不确定。您能检查一下其执行计划并查看数据库中的性能是否还有改进空间吗?也许我们可以调整这条查询，或者添加更多索引。请注意，目前的状况意味着用户每次查看主列表时都会受到影响。
这里承认目前作法中正确的部分(即SQL结果正确)。
1. 其提到合理的怀疑并表示这仅仅是种怀疑。(‘我不确定’。)
2. 其表达了可能的问题来源(‘表扫描’、‘未使用索引’)。
3. 其给出了可行建议(‘检查执行计划’)。
4. 建议潜在的修改方向(‘变更查询’，‘添加索引’)。
5. 其表达了变更在实际应用中的影响(‘用户查看列表时的性能表现’)。
2. 要明确给出质疑的理由
在提供变更建议或者指出问题时，大家注意要明确给出质疑的理由。这样做能够确保其他参与对话的同事根据我们的表述或者自身理解找到正确的解决方案。
这一点非常重要，特别是在大家并没有明确提出预期解决方法的情况下。单纯提出问题并不足以帮助代码编写者了解潜在的问题根源。
以下是负面示例：
这里有拼写错误。
这之所以被归为负面示例，是因为如果对方都没有发现拼写错误，那么很可能再次检查时也发现不了错误。当然，除非这里的错误非常明显。无论如何，最好直接给出正确的拼写。
需要在这里提醒下各位：
“这里”是种很模糊的表达，而且除非开发者已经意识到在代码内添加验证流程的意义所在，否则其很可能直接向这部分代码片段中复制一条验证语句，或者盲目猜测审查者的意图。
以下是正面示例
这里拼写成了“typ”，实际应为“type”。
这里纠正了负面示例：指出错误拼写，并给出正确结果。
还有一点是因代码没有添加验证属性允许任何未授权用户调用。这可能造成安全问题，包括由未经许可的第三方变更系统的内部状态。请添加验证属性以避免发生此类问题。
解释问题细节，帮助开发者理解相关影响并在未来的开发工作中避免其再次发生，最后给出理想的解决建议。
3.需要审查什么
在审查过程中，我们需要考虑的问题多种多样。不过总体来讲，如果答案让您感到别扭，那基本代表着我们发现了一个问题，并需要提供反馈意见以协助将其解决。
然而，同样需要注意的是，大家应该适当放弃争论，特别是在问题并不会影响产品质量的情况下。很明显，一味揪住不太正确的缩进习惯不放，只会让开发者心生怒火。
一般的情况下，大家都会直接审查全部代码后，再审查第二遍乃至第三遍; 当然，我们也可以用清单的方式对各个部分进行分别审查。我根据自己的理解，将代码中的各部分内容进行了重要度排序：
快速检查清单
1. 变更是否可行?
1. 变更能否实现预期效果?也许应当配合项目初始要求进行协同考量。
2. 假设及其约束条件是否适用于这套系统?
3. 如果会对用户造成影响，那么这种影响是否在可接受范围内?(例如向Web应用中添加一套新的大规模JavaScript库。)
4. 变更是否会带来后续成本?(包括服务器、许可乃至处理器资源利用等。)
5. 变更是否合法?(是否避免使用未付费的专利库、来自其它源的代码、非兼容许可乃至未受保护的数据等?
2. 其是否拥有正确的技术导向?
现有设计是否适用于以往规划的解决方案?
变更是否会引入合理的复杂性水平?
是否保证将故障点数量控制在最低程度?
是否具备灵活性?其在未来的产品发展中是否仍提供充分的灵活性空间?
使用更为简单或者适当的解决方案时，是否能够避免引入第三方库及代码?
3. 能否妥善处理错误及意外情况?
1. 如果可以，其是否会被纳入日志记录?
2. 如果可以，其是否会向用户提供正确的提示信息?
3. 如果可以，其是否会拖慢开发团队的工作进度?
4. 错误及意外情况是否得到处理?其是否在正确的代码区段内得到处理
4. 其中是否包含正确的指标数量?
1. 其是否能够指示出有意义的数据?
5. 变更是否安全?
1. 其是否避免披露一切应当受到保护的数据?
2. 其是否会对一切不应被普通存储或传输的数据进行了加密?
3. 其是否对输入内容或者第三方系统进行检查或者验证?
6. 其是否避免引入技术债务?
1. 如果引入了技术债务，这些债务条目是否在代码中注明了引入理由?
2. 这些理由是否无法利用现有有效方法加以解决?
3. 技术债务是否在可接受范围内
7. 其是否经过测试?
1. 输入数据是否接受有效性/白名单/黑名单机制处理?
2. 其是否为相关数据使用了正确的变量大小(例如short amountOfPeopleInStadium就不太适当。)
3. 其是否包含单元测试?这些测试是否有效?
4. 其是否包含集成测试?这些测试是否有效?
5. 其是否包含自动化测试?这些测试是否有效?
6. 如果要修复一项bug，相关测试能够重现原始bug吗
8. 代码是否具备可读性?
1. 其中是否包含拼写错误或者错别字?
2. 其中是否包含奇怪的缩写?
3. 对于写入内容，其是否可理解且表意明确?(即在说明文档及注释中使用合适的语言及表达。)
4. 注释内容是否有意义及有效
9. 代码是否可维护?
1. 如果其向系统中引入了新“概念”，这部分概念的定义是否在说明文档中有所体现?
2. 其中是否避免使用任何奇怪的代码结构?(即‘炫技型代码’。)
3. 变更与方法的命名与其含义是否相符?(您能否根据其名称了解其含义?)
10. 是否采用正确的编码风格?
1. 变量/方法/类用法
2. 缩进
3. 大括号用法
4. 注释密度
4.以正确方式阅读代码
根据项目类型以及所引入变更的具体特性，我们往往可以通过多种方式实现这种变更。大家可以这样思考：当我们在查找特定错误代码片段时，通常会在头脑中完成调试流程，包括读取从输入到处理的对应代码，并根据调用顺序捋顺其逻辑。这种代码阅读方式能够帮助大家更加明确各组件间的关联及交互方式，但其它一些场景则不太适用这种方式。
自上而下阅读代码能够帮助大家了解其中的抽象含义，并弄清是否需要利用灵活代码以支持不同场景，但这往往不能让我们很快掌握代码之间依赖性的冗余或者缺失问题——因此这种作法并不可取。
在查看不同的模块或者命名空间时，大家可以关注其子系统交互及组织方式，这能帮助大家找到那些常见的设计与架构问题，但往往会漏掉具体实现细节。
总之，我们应确保在审查代码时从多个角度考虑问题，同时变换立场及顺序反复检查。
5.以不会导致误解的方式进行反馈意见讨论
作为审查人员，我们的反馈意见往往能够直接被传达至初始开发者处。另外，其中某些意见可能相当具体，而另一些则较为开放，意味着需要引入更多开发者进行广泛讨论。如果您的反馈意见单纯以大篇幅文本形式体现，那么往往很难在不引发误解的前提下顺利进行讨论。
我个人比较喜欢GitHub及Bitbucket等服务中的评论功能。这些意见会分行清晰显示，用户能够清晰掌握其相关上下文，并针对反馈中的特定部分进行具体讨论。更重要的是，如果文件内容发生变更(这在反馈过程中非常常见)，那么对之前代码的反馈会被隐藏起来，这意味着过期的反馈意见将不再被纳入代码审查流程。
GitHub目前采用的代码审查方案是，我们可以将自己的全部信息进行队列排布，将其作为审查内容的一部分进行发送，并随后进行批准或者拒绝。微软提供的TFS在线服务也采取类似的机制。这种方法非常有效，我们能够随时发现人们对代码的错误以及改进空间的留言，并在之后进行针对性修改。不过在采取这种方式时，请务必在发送前对全部评论内容进行认真审校。
6.避免“挤牙膏”
要尽可能提供完整的审查意见，而非像挤牙膏般发现一点问题就通知开发者进行修改。无论大家提供的是单一文本还是拆分式注释，都要尽可能一次性提供全部信息，这样开发者(也包括我们自己)才能更为充分地利用时间。对于开发者来说，到处乱窜进行代码修改绝不是什么舒适的体验，而由此产生的怨气最终都会被归结于您。
有时候人们会通过邮件发送代码审查反馈。在这种情况下，邮件的结构就变得非常重要，因为良好的格式能让对方逐行加以回答，我们也可以在得到合理答复后将不必要的部分删去。
7.要有礼貌
在提交反馈意见时，千万不要抱有“这就是个错误”的心态——即使事实确实如此。请始终坚持“这里还有改进空间/这里应该加以改进”的态度; 另外，除非您百分之百确定，否则请以疑问的口吻提出意见。请记住，接受审查的对象是人而不是机器; 第二，他们已经尽可能做好这份工作了。即使出于技术、知识、经验或者时间所限而导致开发者拿出了无法接受的代码成果，也请坚信一点，他们已经为项目的推进贡献出了大量精力。
Linus Torvalds式的粗口与责骂虽然读起来颇具趣味，但对于交流对象而言却是种很深的侮辱与伤害。另外，为什么非要给自己树敌呢?毕竟老话说得好，和气才能生财嘛。
原文标题：How to perform a good code review
【51CTO译稿，合作站点转载请注明原文译者和出处为51CTO.com】
联系客服
