代码审查是消灭Bug最重要的方法之一,这些审查在大多数时候都特别奏效。由于代码审查本身所针对的对象,就是俯瞰整个代码在测试过程中的问题和Bug。并且,代码审查对消除一些特别细节的错误大有裨益,尤其是那些能够容易在阅读代码的时候发现的错误,这些错误往往不容易通过机器上的测试识别出来。本文就常见的Java代码中容易出现的问题提出一些建设性建议,以便您在审查代码的过程中注意到这些常见的细节性错误。 gb.f%rlZ`
T&?w"T2y
@)9REA(U
通常给别人的工作挑错要比找自己的错容易些。别样视角的存在也解释了为什么作者需要编辑,而运动员需要教练的原因。不仅不应当拒绝别人的批评,我们应该欢迎别人来发现并指出我们的编程工作中的不足之处,我们会受益匪浅的。 p@#]mVJ>9
]b}B~jD
IO}+[%ptc*
^$e0t;W=
正规的代码审查(code inspection)是提高代码质量的最强大的技术之一,代码审查?由同事们寻找代码中的错误?所发现的错误与在测试中所发现的错误不同,因此两者的关系是互补的,而非竞争的。 A;odVaH7
Bx+d3
1v;'d1Hg;
VMaS;)0f@
如果审查者能够有意识地寻找特定的错误,而不是靠漫无目的的浏览代码来发现错误,那么代码审查的效果会事半功倍。在这篇文章中,我列出了11个Java编程中常见的错误。你可以把这些错误添加到你的代码审查的检查列表(checklist)中,这样在经过代码审查后,你可以确信你的代码中不再存在这类错误了。 \M+MDT&
u@AI&[Z
q_-ma_F#s
BqQ] x'AF
一、常见错误1# :多次拷贝字符串 )\8URc|J
- oU@D
\^Ep>Pq`]
tJff+n>
测试所不能发现的一个错误是生成不可变(immutable)对象的多份拷贝。不可变对象是不可改变的,因此不需要拷贝它。最常用的不可变对象是String。 1Wv{xML"
dAL0.>|`0
aD+0\I[x
])eOa%
如果你必须改变一个String对象的内容,你应该使用StringBuffer。下面的代码会正常工作: *U
M!(
_Q>
"\_,
+Dd"41
{nTG~d
String s = new String ("Text here"); 3Hs$]nQ_X
xsYE=^uv
]Qd{ '}+
hXNH"0VCV
但是,这段代码性能差,而且没有必要这么复杂。你还可以用以下的方式来重写上面的代码: itw{;j
`Uv)Sf{
;8BA~,4l
*vb ^N0P
String temp = "Text here"; 9MT? .q
String s = new String (temp); F#B5sLNb
<3lUV7!
>jv\Qh
,6DD=w 0r
但是这段代码包含额外的String,并非完全必要。更好的代码为: N ,+(>?yE
R0vww_fz
\<X2ns@Tf
p'gb)nI
String s = "Text here"; sllzno2bU
3B;}j/h2
wDMjk2YN
&-=K:;x
二、常见错误2#: 没有克隆(clone)返回的对象 l~rb]6E
:\mRtVH
C-Ig_Nc
L@nebT;\'
封装(encapsulation)是面向对象编程的重要概念。不幸的是,Java为不小心打破封装提供了方便??Java允许返回私有数据的引用(reference)。下面的代码揭示了这一点: iTu0T!4F
sXiv,
3?@?-q2g
.1LPlZ
import java.awt.Dimension; q4k.f_{
/***Example class.The x and y values should never*be negative.*/ 8bt53ta
public class Example{ mL$f[
private Dimension d = new Dimension (0, 0); Ib6(Bp9.L
public Example (){ } t7bqk!6hM\
~,gLplpG0
/*** Set height and width. Both height and width must be nonnegative * or an exception is thrown.*/ r1;e 0\?`
public synchronized void setValues (int height,int width) throws IllegalArgumentException{ )&,K94
if (height < 0 || width < 0) tFiR!f)
throw new IllegalArgumentException(); (i]Z|@|)
d.height = height; E M Q4yK
d.width = width; LWD#a~
} f`?0WJ(M
U(*yL-
public synchronized Dimension getValues(){ {fU?idY)c
// Ooops! Breaks encapsulation `|EH[W&y
return d; nvt$F%+
} Yb6q))Y
} |1Hc&
! Mo`^t
Y@%`ZPJ
G6Nb{m
Example类保证了它所存储的height和width值永远非负数,试图使用setValues()方法来设置负值会触发异常。不幸的是,由于getValues()返回d的引用,而不是d的拷贝,你可以编写如下的破坏性代码: MMgx|"
DsGI/c
OKAkl
mxp Y&Y
Example ex = new Example(); j5^-.sEEw
Dimension d = ex.getValues(); ")%r}:0
d.height = -5; f4"4ZVcr
d.width = -10; smup,RNZRX
vq>l>as9O
"Pj}E=!k
wa ky<w,
现在,Example对象拥有负值了!如果getValues() 的调用者永远也不设置返回的Dimension对象的width 和height值,那么仅凭测试是不可能检测到这类的错误。 mmP U
si(cOCj/
*_"u)<J
TzKK;(GX
不幸的是,随着时间的推移,客户代码可能会改变返回的Dimension对象的值,这个时候,追寻错误的根源是件枯燥且费时的事情,尤其是在多线程环境中。 /g76Hw>H
p/*"4-S
O('Nn]wo~9
x=*L-
更好的方式是让getValues()返回拷贝: V LdB_r3lQ
J@]k%h
jg_n 7
;GOz>pg
public synchronized Dimension getValues(){ :=fvZA WD
return new Dimension (d.x, d.y); *||d\peQ
} c1%rV`)]
~pz FZ7n4
K)N 0,Qwu
8\Hr5FqB(
现在,Example对象的内部状态就安全了。调用者可以根据需要改变它所得到的拷贝的状态,但是要修改Example对象的内部状态,必须通过setValues()才可以。 XUSvhr$|
o~1 Kp!U
4l @)K9F
WG5W0T_
三、常见错误3#:不必要的克隆 v}[dnG
ZnfNQl[
gQouOjfP
v(O=IUa
我们现在知道了get方法应该返回内部数据对象的拷贝,而不是引用。但是,事情没有绝对: _0(7GE13p
s'u(B]E
9wh2f7k
ir[jCea,
/*** Example class.The value should never * be negative.*/ s>%Pd7:
public class Example{ PDP[5q r
private Integer i = new Integer (0); H%}IuHhN)
public Example (){ } -F1-
e+=
/:[2'_Xl
/*** Set x. x must be nonnegative* or an exception will be thrown*/ |vILp/"9=W
public synchronized void setValues (int x) throws IllegalArgumentException{ 'q{733o
if (x < 0) J|~26lG
throw new IllegalArgumentException(); q_t4OrLr=
i = new Integer (x); uf' 4'
} 8;" *6vHZ
jH*)%n5,\
public synchronized Integer getValue(){ N1x@-/xa|
// We can’t clone Integers so we makea copy this way. )=^w3y
return new Integer (i.intValue()); t"AzI8O
} jirbUl
} :}q\tNY<
0q6I;$H
^g>1U5c
;#k-)m%
这段代码是安全的,但是就象在错误1#那样,又作了多余的工作。Integer对象,就象String对象那样,一旦被创建就是不可变的。因此,返回内部Integer对象,而不是它的拷贝,也是安全的。 :`Az/U[
?mYYt]R
K_/B?h
{nMAm/kyj
方法getValue()应该被写为: ;4#D,z lO^
y,Q5;$w8
P0GeZ02]
mpay^.(%
public synchronized Integer getValue(){ uCP>y6I
// ’i’ is immutable, so it is safe to return it instead of a copy. =o=1"o[
return i; t?)pl2!A
} olYsT**'
lmCZ8 j(FF
Sp SnoVI
O#kq^C}
Java程序比C++程序包含更多的不可变对象。JDK 所提供的若干不可变类包括: Rf"Mr: ^
pW?&J>\6
3f76kl(&
u{f*
M,k
?Boolean %_M2N.n
?Byte k(s;,B\
?Character 0E/:|k
?Class v3RcwySk
?Double O&Z'r
?Float LMl~yqM
?Integer n!ok?=(kQ
?Long z; }6f
?Short /Z%>ArAx
?String
_^t-9
?大部分的Exception的子类 N2,D:m\
uGJ"!K
%i0\1hhV<
,^s
四、常见错误4# :自编代码来拷贝数组 mDMt5(.
+8P,s[0<R_
^ @=^;nB
)NW6?Pu"
Java允许你克隆数组,但是开发者通常会错误地编写如下的代码,问题在于如下的循环用三行做的事情,如果采用Object的clone方法用一行就可以完成: W?RE'QV8
K%g;NW
SW?p?<
\E4B&!m
public class Example{ 0Bolv_e
private int[] copy; Y:QD
/*** Save a copy of ’data’. ’data’ cannot be null.*/ r.3KPiYK
public void saveCopy (int[] data){ HK=[U9 o?
copy = new int[data.length]; 6LUC!Sh
for (int i = 0; i < copy.length; ++i) tbDoP
Y
copy = data; ]F*3"y?)2
} 6j9)/ HP
} %!j:fJ()
^CT&0
=_TaA(79
j2n,f7hl.
这段代码是正确的,但却不必要地复杂。saveCopy()的一个更好的实现是: m~l
F`?
fQLax
xK%=
$0Yh!L ?\
void saveCopy (int[] data){ 7,$z;Lr0S
try{ ]o/|na*
copy = (int[])data.clone(); 83ipf"]*
}catch (CloneNotSupportedException e){ fZWGn6$
// Can’t get here. ZU2laqa_
} WOytxE
} $ChK]v
6C
JC;^--0(z
]> !<G8=N
e(B9liXM
如果你经常克隆数组,编写如下的一个工具方法会是个好主意: QL7>;t;
(&\aA 0-}H
c9Es%@]
in%;Eqk
static int[] cloneArray (int[] data){ ^s/
try{ ZXb0Y2AVx
return(int[])data.clone(); q }C+tn"\
}catch(CloneNotSupportedException e){ \>/M .2
// Can’t get here. -`c:}m
} $6>?;
} tx7~SUr
>um!Eo
+{`yeZ9S
u62 )QJE
这样的话,我们的saveCopy看起来就更简洁了: E">T*ao
xBnbF[
uV6g[J
'C+;r?1!h
void saveCopy (int[] data){ $A\m>*@
copy = cloneArray ( data); =(r*
5vd
} Tp%(I"H'_;
ztM<J+
ZDLMMXx>
WT3gNNx|
五、常见错误5#:拷贝错误的数据 %kI}
[6J_
0Ce]V,i6C>
dG'SZ&<
EmVuwphv
有时候程序员知道必须返回一个拷贝,但是却不小心拷贝了错误的数据。由于仅仅做了部分的数据拷贝工作,下面的代码与程序员的意图有偏差: tV;%J4E'
}E<^gAh}
9|r* pK[
8s}J!/2
import java.awt.Dimension; US&B!Q:v
/*** Example class. The height and width values should never * be >%b\yl%0
negative. */ ;]D(33)(
public class Example{ ]L_w$ev'
static final public int TOTAL_VALUES = 10; J@"utY6N
private Dimension[] d = new Dimension[TOTAL_VALUES]; KJhN J
public Example (){ } 7G2PMe;$m
Jcf"#u-Q/
/*** Set height and width. Both height and width must be nonnegative * or an exception will be thrown. */ nY-* i!H
public synchronized void setValues (int index, int height, int width) throws IllegalArgumentException{ _cI_#
if (height < 0 || width < 0) }6zbT-i
throw new IllegalArgumentException(); n[+'OU[
if (d[index] == null) 6Y*;{\Rd
d[index] = new Dimension(); x4?10f(9=
d[index].height = height; +JdZPb
d[index].width = width; GRYe<