搜索
一、概述代码审查（CodeReview）是消灭Bug最重要的方法之一，这些审查在大多数时候都特别奏效。由于代码审查本身所针对的对象，就是俯瞰整个代码在测试过程中的问题和Bug。并且，代码审查对消除一些特别细节的错误大有裨益，尤其是那些能够容易在阅读代码的时候发现的错误，这些错误往往不容易通过机器上的测试识别出来。1.1主要工作1、发现代码中的bug;2、从代码的易维护性、可扩展性角度考察代码的质量，提出修改建议。3、是否符合java开发规范和代码审核检查表1.2基本流程1、代码编写者和代码审核者坐在一起，由代码编写者按照UC（UseCase）依次讲解自己负责的代码和相关逻辑，从表现层-持久层;2、代码审核者在此过程中可以随时提出自己的疑问，同时积极发现隐藏的bug;对这些bug记录在案。3、代码讲解完毕后，代码审核者给自己安排几个小时再对代码审核一遍。代码需要一行一行静下心看。同时代码又要全面的看，以确保代码整体上设计优良。4、代码审核者根据审核的结果编写“代码审核报告”，“审核报告”中记录发现的问题及修改建议，然后把“审核报告”发送给相关人员。5、代码编写者根据“代码审核报告”给出的修改意见，修改好代码，有不清楚的地方可积极向代码审核者提出。6、代码编写者bugfix完毕之后给出反馈。7、代码审核者把CodeReview中发现的有价值的问题更新到代码审核检查表的文档中，对于特别值得提醒的问题可群发email给所开发人员。1.3责任代码编写者，代码审核者共同对代码的质量承担责任。这样才能保证CodeReview不是走过场，其中代码编写者承担主要责任，代码审核者承担次要责任。二、java代码审查检查表重要性激活级别检查项总计命名重要20命名规则是否与所采用的规范保持一致？20是否遵循了最小长度最多信息原则？重要50has/can/is前缀的函数是否返回布尔型？注释重要10注释是否较清晰且必要？重要Y10复杂的分支流程是否已经被注释？10距离较远的}是否已经被注释？10非通用变量是否全部被注释？重要Y50函数是否已经有文档注释？（功能、输入、返回及其他可选）10特殊用法是否被注释？声明、空白、缩进20每行是否只声明了一个变量？（特别是那些可能出错的类型）重要40变量是否已经在定义的同时初始化？重要40类属性是否都执行了初始化？20代码段落是否被合适地以空行分隔？Y20是否合理地使用了空格使程序更清晰？20代码行长度是否在要求之内？20折行是否恰当？语句/功能分布/规模20包含复合语句的{}是否成对出现并符合规范？20是否给单个的循环、条件语句也加了{}？20if/if-else/if-elseif-else/do-while/switch-case语句的格式是否符合规范？40单个变量是否只做单个用途？重要20单行是否只有单个功能？（不要使用；进行多行合并）重要40单个函数是否执行了单个功能并与其命名相符？Y20操作符＋＋和——操作符的应用是否复合规范？规模重要20单个函数不超过规定行数？重要100缩进层数是否不超过规定？重要100是否已经消除了所有警告？重要Y40常数变量是否声明为final？重要80对象使用前是否进行了检查？重要80局部对象变量使用后是否被复位为NULL？重要70对数组的访问是否是安全的？（合法的index取值为[0,MAX_SIZE-1]）。重要20是否确认没有同名变量局部重复定义问题?20程序中是否只使用了简单的表达式？重要Y20是否已经用（）使操作符优先级明确化？重要Y20所有判断是否都使用了（常量==变量）的形式？80是否消除了流程悬挂？重要80是否每个if-elseif-else语句都有最后一个else以确保处理了全集？重要80是否每个switch-case语句都有最后一个default以确保处理了全集？80for循环是否都使用了包含下限不包含上限的形式？（k=0;kMAX）重要40XML标记书写是否完整，字符串的拼写是否正确?40对于流操作代码的异常捕获是否有finally操作以关闭流对象?20退出代码段时是否对临时对象做了释放处理?重要40对浮点数值的相等判断是否是恰当的？（严禁使用==直接判断）可靠性（函数）重要Y60入口对象是否都被进行了判断不为空？重要Y60入口数据的合法范围是否都被进行了判断？(尤其是数组)重要Y20是否对有异常抛出的方法都执行了try...catch保护？重要Y80是否函数的所有分支都有返回值？重要50int的返回值是否合理？（负值为失败，非负值成功）20对于反复进行了int返回值判断是否定义了函数来处理？60关键代码是否做了捕获异常处理?重要60是否确保函数返回CORBA对象的任何一个属性都不能为null?重要60是否对方法返回值对象做了null检查，该返回值定义时是否被初始化?重要60是否对同步对象的遍历访问做了代码同步?重要80是否确认在对Map对象使用迭代遍历过程中没有做增减元素操作？重要60线程处理函数循环内部是否有异常捕获处理，防止线程抛出异常而退出?20原子操作代码异常中断，使用的相关外部变量是否恢复先前状态?重要100函数对错误的处理是恰当的？可维护性重要100实现代码中是否消除了直接常量？（用于计数起点的简单常数例外）20是否消除了导致结构模糊的连续赋值？（如a=(b=d+c)）20是否每个return前都要有日志记录?20是否有冗余判断语句？（如：if(b)returntrue;elsereturnfalse;）20是否把方法中的重复代码抽象成私有函数?三、Java代码审查的常见错误3.1常见错误1#：多次拷贝字符串测试所不能发现的一个错误是生成不可变(immutable)对象的多份拷贝。不可变对象是不可改变的，因此不需要拷贝它。最常用的不可变对象是String。如果你必须改变一个String对象的内容，你应该使用StringBuffer。下面的代码会正常工作：Strings=newString(Texthere);但是，这段代码性能差，而且没有必要这么复杂。你还可以用以下的方式来重写上面的代码：Stringtemp=Texthere;Strings=newString(temp);但是这段代码包含额外的String，并非完全必要。更好的代码为：Strings=Texthere;3.2常见错误2#：没有克隆(clone)返回的对象封装(encapsulation)是面向对象编程的重要概念。不幸的是，Java为不小心打破封装提供了方便——Java允许返回私有数据的引用(reference)。下面的代码揭示了这一点：importjava.awt.Dimension;/***Exampleclass.Thexandyvaluesshouldnever*benegative.*/publicclassExample{privateDimensiond=newDimension(0,0);publicExample(){}/***Setheightandwidth.Bothheightandwidthmustbenonnegative*oranexceptionisthrown.*/publicsynchronizedvoidsetValues(intheight,intwidth)throwsIllegalArgumentException{if(height0||width0)thrownewIllegalArgumentException();d.height=height;d.width=width;}publicsynchronizedDimensiongetValues(){//Ooops!Breaksencapsulationreturnd;}}Example类保证了它所存储的height和width值永远非负数，试图使用setValues()方法来设置负值会触发异常。不幸的是，由于getValues()返回d的引用，而不是d的拷贝，你可以编写如下的破坏性代码：Exampleex=newExample();Dimensiond=ex.getValues();d.height=-5;d.width=-10;现在，Example对象拥有负值了！如果getValues()的调用者永远也不设置返回的Dimension对象的width和height值，那么仅凭测试是不可能检测到这类的错误。不幸的是，随着时间的推移，客户代码可能会改变返回的Dimension对象的值，这个时候，追寻错误的根源是件枯燥且费时的事情，尤其是在多线程环境中。更好的方式是让getValues()返回拷贝：publicsynchronizedDimensiongetValues(){returnnewDimension(d.x,d.y);}现在，Example对象的内部状态就安全了。调用者可以根据需要改变它所得到的拷贝的状态，但是要修改Example对象的内部状态，必须通过setValues()才可以。3.3常见错误3#：不必要的克隆我们现在知道了get方法应该返回内部数据对象的拷贝，而不是引用。但是，事情没有绝对：/***Exampleclass.Thevalueshouldnever*benegative.*/publicclassExample{privateIntegeri=newInteger(0);publicExample(){}/***Setx.xmustbenonnegative*oranexceptionwillbethrown*/publicsynchronizedvoidsetValues(intx)throwsIllegalArgumentException{if(x0)thrownewIllegalArgumentException();i=newInteger(x);}publicsynchronizedIntegergetValue(){//Wecan’tcloneIntegerssowemakeacopythisway.returnnewInteger(i.intValue());}}这段代码是安全的，但是就象在错误1#那样，又作了多余的工作。Integer对象，就象String对象那样，一旦被创建就是不可变的。因此，返回内部Integer对象，而不是它的拷贝，也是安全的。方法getValue()应该被写为：publicsynchronizedIntegergetValue(){//’i’isimmutable,soitissafetoreturnitinsteadofacopy.returni;}Java程序比C++程序包含更多的不可变对象。JDK所提供的若干不可变类包括：·Boolean·Byte·Character·Class·Double·Float·Integer·Long·Short·String·大部分的Exception的子类3.4常见错误4#：自编代码来拷贝数组Java允许你克隆数组，但是开发者通常会错误地编写如下的代码，问题在于如下的循环用三行做的事情，如果采用Object的clone方法用一行就可以完成：publicclassExample{privateint[]copy;/***Saveacopyof’data’.’data’cannotbenull.*/publicvoidsaveCopy(int[]data){copy=newint[data.length];for(inti=0;icopy.length;++i)copy[i]=data[i];}}这段代码是正确的，但却不必要地复杂。saveCopy()的一个更好的实现是：voidsaveCopy(int[]data){try{copy=(int[])data.clone();}catch(CloneNotSupportedExceptione){//Can’tgethere.}}如果你经常克隆数组，编写如下的一个工具方法会是个好主意：stati