同行评审-代码审查参考文档

代码审查参考文档

代码审查（code review）是保证软件质量的一个重要环节，通过审查代码能够发现代码中可能存在的问题并给予纠正，这些问题可能包括设计上的、实现上的或者编程风格等多方面。本文档通过列举代码编写过程中的一些常见的细节问题，为代码审查环节提供参考。

Java代码

一、对象和变量

1.存在未被使用的变量

Eclipse会自动用下划线标出

2.对象的重复创建

这是系统中普遍存在的问题，比如：

public class PrtGrpEndorsementBL {

private GlobalInput mGlobalInput = new GlobalInput();

private boolean getInputData(VData cInputData) {

mGlobalInput = (GlobalInput) cInputData.getObjectByObjectName( "GlobalInput", 0);

return true;

}

}

这里mGlobalInput对象属于重复创建，因为在getInputData方法里会对它进行赋值，mGlobalInput使用的应该是从jsp页面传入的对象，所以改为private GlobalInput mGlobalInput = null;

又如：

String msg = "";

if (..) {

msg = "A";

}

else {

msg = "B";

}

这里msg同样属于重复创建，改为String msg = null;

3.变量的作用域

Java的局部变量可以定义在函数的任何位置，有部分由c转学java的程序员习惯将变量都定义在函数的顶部，因为在c里只能那样定义。但实际上变量的作用域越短程序的内聚性就越高，耦合性也更低，程序更容易理解，因此在java里应该在使用前才定义变量。

4.局部变量的危害

定义过多的不必要的局部变量是造成系统难以维护的原因之一，因为每增加一个局部变量我们就要先化时间去理解这个局部变量的意思，因此我们要减少局部变量的使用。用函数的返回值来替代局部变量是一种有效的办法，这就需要我们用重构的方式从大的函数中提出小的函数，用小函数的返回值来替代原有的局部变量。把大函数分解本身也可以降低程序的耦合度。

二、常量

1.硬编码，将代码写死

比较严重的情况是将险种代码写死在程序中，比如：

if ("21301".equals(tLCGrpPolSchema.getRiskCode())) {

...

}

这里将“21301”写死会为系统的维护和升级带来很大困难，最好是以险种定义的形式去动态获取险种代码。如果实在描述有困难，可以考虑建立一个Constants常量类，将其以常量的形式定义，这样以后维护只需改动一处即可。

业务系统中存在不少的状态标志，比如AppFlag的状态”0”为未签单，”1”为已签单，”2”为保全增人。虽然这些状态的意义日后已没有改动的可能，但直接将”0”、”1”、”2”写在程序里会导致程序可读性差，因此我们同样可以将其定义在常量类中，并加上注释，这样我们不必每次都去翻看文档或pdm，看常量类里的注释就行了。

2.禁用常量接口

Java的常量有一种不好的用法，就是将常量定义在Inferface中，这种方使定义的常量在使用时可以省去类名前缀，但会为以后的维护带来麻烦。因此不要试图用继承的方式去使用常量。

三、数组

1.数组的定义

数组的定义采用String[] str = null; 的形式，而不是String str[] = null;

2.数组越界

数组越界是比较常见的错误，比如：

private void dealCont() {

LCContDB tLCContDB = new LCContDB();

tLCContDB.setGrpContNo(mGrpContNo);

tLCContSet = tLCContDB.query();

for (int i = 0; i < tLCContSet.size(); i++) {

LCContSchema tLCContSchema = tLCContSet.get(i);

LCPolDB tLCPolDB = new LCPolDB();

tLCPolDB.setContNo(tLCContSchema.getContNo());

LCPolSet tLCPolSet = tLCPolDB.query();

for (int j = 0; j < tLCPolSet.size(); j++) {

LCPolSchema tLCPolSchema = tLCPolSchema.get(i);

...

}

}

}

上面的程序就会产生数组越界，应该把tLCPolSchema.get(i);中的i改为j

数组越界的错误在编译时不会报错，直到程序运行时才会被发现。有一种方法可以避免此bug的产生，程序改为：

private void dealCont() {

LCContDB tLCContDB = new LCContDB();

tLCContDB.setGrpContNo(mGrpContNo);

tLCContSet = tLCContDB.query();

for (int i = 0; i < tLCContSet.size(); i++) {

dealPol(tLCContSet.get(i));

}

}

private void dealPol(LCContSchema tLCContSchema) {

LCPolDB tLCPolDB = new LCPolDB();

tLCPolDB.setContNo(tLCContSchema.getContNo());

LCPolSet tLCPolSet = tLCPolDB.query();

for (int i = 0; i < tLCPolSet.size(); i++) {