1、敏捷开发中的代码检视整理人：李名俊创建日期：2011-03-05iSoftStone IT Co., Ltd.软通动力信息技术有限公司All rights reserved版权所有 侵权必究1 敏捷开发中 Code Review 的目的及内容做任何事情，首先要清晰为什么要做，才能有目标和动力把事情做得更好，Code Review 也是如此。只有清晰明确了敏捷团队进行 CodeReview 的动机，才能以此为方向开展后续工作。下面我们推荐的敏捷开发中常见的 Code Review 的目的：1.1 设计合理性 Review敏捷开发中崇尚 Code is design，对开发人员提出了比以往更高的要
2、求，即需要开发人员不断地重构出合理的设计。所以敏捷开发中的 Code Review 也需要承担一部分“结对设计”和“设计把关”的职责。这部分的 Code Review 包括：设计的合理性（如实现方法，数据结构，设计模式，扩展性考虑等），是否存在大量重复代码和其他组件是否有重复的代码，包结构设计是否合理等。笔者了解的一些项目中， 进行敏捷开发后， 提高了开发效率， 但是设计的质量却下降了。如 Repeat Yourself 的现象（特别是跨组件之间的 Repeat Yourself 现象）；更有甚者，在笔者看到一个某银行的应用中（不是国内的），数据库连接和操作是直接在 JSP中写 SQL 语句。
3、像这些 Bad Design 的例子还是很多的。这些在重构的时候应该由开发人员解决。但考虑到不同开发人员之间技术功底不一，很有必要在 Code Review 阶段进行 Review 和讨论。1.2 互为 Backup这是很容易被忽略，但是又很重要的一个 Code Review 的目的。我们知道，敏捷开发中强调高质量的代码胜过详细的文档，所以某种程度上来说 Code is Document。敏捷开发中的代码承担了一部分 Document 的职责，即传递技术的作用。Code Review 中，Review 的开发人员了解代码的设计和实现，传递了技术，开发人员互为 Backup，方便后期的维护，也减
4、少了项目风险。1.3 分享知识、设计、技术这也是很容易被忽略的一个很重要的目的。敏捷开发是一个中央集中控制到个体发挥积极性的过程，中央集中控制的优点就是有统一的视图和控制，经常开大会，开长会，这样知识和经验也较容易集中。敏捷开发中，分散在两个 Scrum Team 的开发人员之间，如果没有好的机制，相互沟通也会相对较少，造成知识和好的经验无法在整个团队传播。笔者参加的项目中就碰到了类似情况， 当时我们整个团队分成三个 Scrum Team，其中一个 Scrum Team 负责一个 Eclipse 工具的开发， 其中用到的一些功能和知识在其他ScrumTeam 上以前都有涉及过。当时负责开发的同
5、事非常优秀而且能力突出，但由于不知道其他 Scrum Team 同事有这方面的经验，没有很好地分享以往好的经验和知识，以至于最后导致浪费了一些学习的成本。Code Review 是一个学习和享受的过程，一个开发人员的能力有限，而 Code Review正是这样的一种机制，让好的知识、设计在团队中分享，实现整体团队的成长和整体的效益最大化。1.4 代码可读性如上所说，敏捷开发中强调高质量的代码胜过冗余的文档，所以 Code 某种程度上是Document。敏捷开发中，代码的要求不止是能运行功能正确的代码，而是有了更高的要求，即 Codefor maintenance。可维护的代码，需要清晰，可读性
6、强，这里可读性代码检查不是指代码格式（代码格式可以通过工具检查出），而是指代码语义。在笔者的文章软件可消费性设计中有一些这方面的讨论和建议。1.5 Code 中的“地雷区”Review代码中的逻辑，除了业务逻辑，还应该包括技术逻辑。技术逻辑就是实现逻辑， 比如数据库连接打开是否忘记关闭，是否正确使用线程，Exception 处理，密码是否加密存储等。我把这些最常出现错误的地方，而且是测试不容易发现的地方，称为 Code 中的“地雷区”。这些“地雷区”在 Code Review 中是值得花费一些时间进行维护和检查的。建议，在整个团队中维护并共享“地雷区”注意事项列表，以及统一的处理方式和机制。并
7、在编码和 Code Review 过程中都按照团队的最佳实践进行。1.6 发现代码中的业务逻辑错误业务逻辑指的是代码开发的功能是否符合业务需求，如一个加法函数，检查其是否真的实现了加法的功能。笔者了解的一些敏捷团队中，把发现代码的业务逻辑错误当做目标和内容，但往往效果都不是很好，基本都是从形式上泛泛检查一番。原因有两个：1业务逻辑的检查是从需求到代码的全方位检查，需要花费大量时间，投入产出比失衡。2业务逻辑的检查和业务需求紧密关联，已经超出了检查人员的能力范围（一般Code Review 是开发人员，不是业务人员）。笔者认为，发现逻辑错误，不应该是 Code Review 的目的和内容。应该是
8、 Unit Test，功能测试，集成测试的目的。从投入产出比考虑，不应该花费太多时间在 Code Review 阶段去进行逻辑错误检查。2 敏捷开发中不推荐的 Code Review 的目的及内容下面还有一些常见的 Code Review 目的和内容被很多团队广泛使用，但作者认为这些并不是敏捷开发中的主要目的和内容，团队应该把时间花费在重要的目的和内容上，而不应该投入精力在下面的这些 Code Review 目的和内容上。2.1 发现性能问题有些团队把性能问题，也作为 Code Review 的目的和内容之一，然后提出一些如String 应该使用 StringBuilder，而不能使用+，类似
9、这样的看似有用其实无用建议。笔者认为，性能问题是需要量化的衡量和精确定位， 很难通过 Code Review 检查出来。而一些粗浅的性能问题可以通过一些工具方便地扫描出来，而无须花费时间去进行 Code Review。所以笔者认为，开发人员提交的代码，需要是经过工具检查后的代码。而代码审核人员则无须花费时间在性能相关的 Code Review 上。具体的性能问题交给性能测试。2.2 发现开源的授权法律问题开源软件也可以借助一些检查工具， 统一通过工具扫描， 无需在 Code Review 阶段花费时间。2.3 其他问题，如国际化，J2EE Best Practice 等这些问题开发人员可以在提
10、交代码之前通过工具发现和解决， 不是 Code Review 阶段的职责和目的，也无须花费时间去处理。1设计原则（5）：用于面向对象编程的设计原则的规则。2全球化（47）：基于全球化编码最佳实践的规则，有助于确保代码在局部环境中正确地运行。3J2EE 最佳实践（32）：基于最佳的 Java 2 Platform Enterprise Edition（ J2EE）开发实践的规则，以及支持瞄准 IBM WebSphere 服务的 Web 项目的规则；4J2EE 安全性（17）：验证代码符合 J2EE 技术安全性需要的规则；5J2SE 最佳实践（71）：基于最佳的 Java2 Platform St
11、andard Edition （J2SE）开发实践的规则；6J2SE 安全性（9）：验证代码符合 J2SE 技术安全性需要的规则；7命名（2）：关于 Java 代码中命名约定的规则；8性能（26）：加强在 Java 应用程序中提高性能和减少存储器足迹的建议的规则；9私有 API （3）：定位那些不属于 Java 代码的 API 的规则。3 敏捷开发中如何开展 Code Review在清晰明确了敏捷团队进行 Code Review 的目的和内容后，下面介绍如何有效地开展Code Review。3.1 沟通、协作、互助、学习的团队氛围Code Review 中，Review 人员和开发人员不是对立
12、的关系，而是互助、沟通、协作和学习的过程。团队形成互助、互学的气氛，既能互相增长团队的知识和经验，还能把产品做得更好。Code Review 协作过程：a）先由代码的开发人员向检查人员进行大体的介绍，包括设计思想、数据结构、程序代码结构介绍等。b）双方进行讨论、交流。c）检查人员单独进一步进行 Code Review，并记录 Review 结果和建议。d）由检查人员和开发人员一起，检查人员反馈 Code Review 结果，并和开发人员一起讨论改进方法，重构。e）最后把可重用的 Code Review 的经验总结编码规范，或者记录到“地雷区”中。便于整个团队复用经验。开展以上过程可以以开发人员
13、为主，辅助以工具。但无须规定系列的文档、流程、Check List 等，这反而会影响开发人员的积极性。Code Review 是发现问题的过程，同时也是学习和交流过程。需要是灵活、自由、主动的态度，而不是行政上的控制和规章流程。笔者建议：和敏捷开发的核心思想一致，让团队明确 Code Review 的思想、作用和目的内容后，充分发挥个体的积极性和学习分享的动力。随时随地地进行 Code Review，讨论，重构，改进。3.2 增量式 Review大家都知道，软件开发中存在长鞭效应，即一个问题越在后期发现造成的影响会越大，Code Review 也是软件的开发过程中， 应该阶段性地进行 Code
14、 Review，而不是等到所有代码都开发完毕后再做一次性的 Code Review。那时如果发现问题，造成的改动成本比增量式的检查来的大得多。笔者了解的一些开发团队，他们在软件开发完毕，并测试后，才临时确定 Code Review 的人员，然后再安排半天左右的时间进行 Code Review。结果尽管发现一些结构或设计方面问题，但由于修改成本大，也无法进行改进。正确的方式是，在早期就参与设计开发过程，抱着互助、沟通、协作、学习的思想，阶段性的参与讨论、学习并贡献自己的意见。具体 Review 的频率、次数则可以由开发人员抱着主动、积极的态度，按照敏捷的思想自己去把握决定。3.3 利用工具进行
15、Code Inspection有很多的工具可以辅助 Code Review ：1如代码格式检查 Checkstyle 工具，检查如过大的类、太长的方法和未使用的变量等这样违反编程规范的问题。2RAD 中的 Software Analyzer 工具，可以基于规则进行国际化、J2EE 最佳实践、性能、安全等检查。3CSAR，用于扫描代码检查开源软件等。4JDepend，可以检查包依赖关系。5CPD 工具，Eclipse 的 PMD 插件提供了一项叫做 CPD（或复制粘贴探测器）的功能，用于寻找重复的代码。6Eclipse 的 Metrics 插件，提供了很多有效地查出代码复杂度的功能。辅助以工具和
16、自动化流程，能花很少时间轻松完成很多基本的 Code Inspection 工作。让团队有更多的时间和精力去做更重要的 Code Review。3.4 持续自动化 Code Inspection工具检查可以由开发人员自行检查并修正， 但一种更可持续的做法是自动化的集成工具进行 CodeInspection，可以通过自动化脚本在每日进行 Build 前进行扫描，并呈现报告给相应人员。3.5 Code Review 协作工具为了快速有效地进行人工 Code Review 协作，可以使用诸如 Jupiter 这样的工具辅助进行。可以帮助开发人员有效管理 Code Review 任务、问题、建议等。4
17、 总结Code Review 的核心是：互助，沟通，协作，学习的过程，这是一个美妙而享受的过程，是跨越需求分析、架构设计、编码等各阶段的过程。敏捷团队应该统一达成 Code Review 对产品、对团队、对个人的巨大好处的共识，发挥出个体的积极性，相信会改变“流于形式”的现状，发挥 Code Review 巨大的威力。附 java 代码审查检查表 重要性 激活 级别 检查项总计 命名 重要 20 命名规则是否与所采用的规范保持一致？20 是否遵循了最小长度最多信息原则？重要 50 has/can/is 前缀的函数是否返回布尔型？注释 重要 10 注释是否较清晰且必要？重要 Y 10 复杂的分支
18、流程是否已经被注释？10 距离较远的是否已经被注释？10 非通用变量是否全部被注释？重要 Y 50 函数是否已经有文档注释？（功能、输入、返回及其他可选）1 特殊用法是否被注释？0声明、空白、缩进 20 每行是否只声明了一个变量？（特别是那些可能出错的类型）重要 40 变量是否已经在定义的同时初始化？重要 40 类属性是否都执行了初始化？20 代码段落是否被合适地以空行分隔？Y 20 是否合理地使用了空格使程序更清晰？20 代码行长度是否在要求之内？20 折行是否恰当？语句/功能分布/规模 20 包含复合语句的是否成对出现并符合规范？20 是否给单个的循环、条件语句也加了？20 if/if-e
19、lse/if-else if-else/do-while/switch-case 语句的格式是否符合规范？40 单个变量是否只做单个用途？重要 20 单行是否只有单个功能？（不要使用；进行多行合并）重要 40 单个函数是否执行了单个功能并与其命名相符？Y 20 操作符和 操作符的应用是否复合规范？规模 重要 20 单个函数不超过规定行数？重要 100缩进层数是否不超过规定？可靠性（总则/变量和语句） 重要 100是否已经消除了所有警告？重要 Y 4 常数变量是否声明为 final？0重要 80 对象使用前是否进行了检查？重要 80 局部对象变量使用后是否被复位为 NULL？重要 70 对数组的
20、访问是否是安全的？（合法的 index 取值为0, MAX_SIZE-1）。重要 20 是否确认没有同名变量局部重复定义问题?20 程序中是否只使用了简单的表达式？重要 Y 20 是否已经用（）使操作符优先级明确化？重要 Y 20 所有判断是否都使用了（常量=变量）的形式？80 是否消除了流程悬挂？重要 80 是否每个 if-else if-else 语句都有最后一个 else 以确保处理了全集？重要 80 是否每个 switch-case 语句都有最后一个 default 以确保处理了全集？80 for 循环是否都使用了包含下限不包含上限的形式？（k=0; kMAX）重要 40 XML 标记
21、书写是否完整，字符串的拼写是否正确 ?40 对于流操作代码的异常捕获是否有 finally 操作以关闭流对象?20 退出代码段时是否对临时对象做了释放处理?重要 40 对浮点数值的相等判断是否是恰当的？（严禁使用=直接判断）可靠性（函数） 重要 Y 60 入口对象是否都被进行了判断不为空？重要 Y 60 入口数据的合法范围是否都被进行了判断？(尤其是数组)重要 Y 20 是否对有异常抛出的方法都执行了 try.catch 保护？重要 Y 80 是否函数的所有分支都有返回值？重要 50 int 的返回值是否合理？（负值为失败，非负值成功）2 对于反复进行了 int 返回值判断是否定义了函数来处理
22、？060 关键代码是否做了捕获异常处理?重要 60 是否确保函数返回 CORBA 对象的任何一个属性都不能为null?重要 60 是否对方法返回值对象做了 null 检查，该返回值定义时是否被初始化?重要 60 是否对同步对象的遍历访问做了代码同步?重要 80 是否确认在对 Map 对象使用迭代遍历过程中没有做增减元素操作？重要 60 线程处理函数循环内部是否有异常捕获处理，防止线程抛出异常而退出?20 原子操作代码异常中断，使用的相关外部变量是否恢复先前状态?重要 100函数对错误的处理是恰当的？可维护性 重要 100实现代码中是否消除了直接常量？（用于计数起点的简单常数例外）20 是否消除了导致结构模糊的连续赋值？（如 a= (b=d+c )）20 是否每个 return 前都要有日志记录?20 是否有冗余判断语句？（如：if (b) return true; else return false;）20 是否把方法中的重复代码抽象成私有函数?
