代码审查是消灭Bug最重要的方法之一,这些审查在大多数时候都特别奏效。由于代码审查本身所针对的对象,就是俯瞰整个代码在测试过程中的问题和Bug。并且,代码审查对消除一些特别细节的错误大有裨益,尤其是那些能够容易在阅读代码的时候发现的错误,这些错误往往不容易通过机器上的测试识别出来。本文就常见的Java代码中容易出现的问题提出一些建设性建议,以便您在审查代码的过程中注意到这些常见的细节性错误。 Vo; B#lK
B/n/bi8T
%dN',
通常给别人的工作挑错要比找自己的错容易些。别样视角的存在也解释了为什么作者需要编辑,而运动员需要教练的原因。不仅不应当拒绝别人的批评,我们应该欢迎别人来发现并指出我们的编程工作中的不足之处,我们会受益匪浅的。 ZnVx'Y
VY#:IE:T
;#>,eD2u
f]*_]J/
正规的代码审查(code inspection)是提高代码质量的最强大的技术之一,代码审查?由同事们寻找代码中的错误?所发现的错误与在测试中所发现的错误不同,因此两者的关系是互补的,而非竞争的。 qtQB}r8
r'GD
K5ywO8_6`
3SU:Xd(\o
如果审查者能够有意识地寻找特定的错误,而不是靠漫无目的的浏览代码来发现错误,那么代码审查的效果会事半功倍。在这篇文章中,我列出了11个Java编程中常见的错误。你可以把这些错误添加到你的代码审查的检查列表(checklist)中,这样在经过代码审查后,你可以确信你的代码中不再存在这类错误了。 yOQEF\
\dG#hH4ZD
M.loG4r!
>JWW2<
一、常见错误1# :多次拷贝字符串 UojHlTg#bT
f5droys9
-eA3o2'
|K jy4.2
测试所不能发现的一个错误是生成不可变(immutable)对象的多份拷贝。不可变对象是不可改变的,因此不需要拷贝它。最常用的不可变对象是String。 2^TJ_xG~
=64%eF
tI&E@
bB#6Xx
如果你必须改变一个String对象的内容,你应该使用StringBuffer。下面的代码会正常工作: Bp.z6x4
QSNLo_z
YdT-E
r8uc. z2%
String s = new String ("Text here"); t622b?w
|}O9'fyU8
! 54(K6a[
,M)NC%0X
但是,这段代码性能差,而且没有必要这么复杂。你还可以用以下的方式来重写上面的代码: bns([F
R06zca
R'.YE;leBG
&M6cCT]&M
String temp = "Text here"; y9>?
String s = new String (temp); 2|8&=K /
U_/<tWl\[3
_1?
PN8
@NY$.K#]
但是这段代码包含额外的String,并非完全必要。更好的代码为: 4=T>Iy
c/g"/ICs
+T R#
v.aSf`K
String s = "Text here"; )t/[z3rn
5X'com?T
DW,fh8 w
N;.cZp2
二、常见错误2#: 没有克隆(clone)返回的对象 CLU[')H0
9m:qQ1[\
0tzMu#
GsG9;6c+u
封装(encapsulation)是面向对象编程的重要概念。不幸的是,Java为不小心打破封装提供了方便??Java允许返回私有数据的引用(reference)。下面的代码揭示了这一点: z+J4XpX0,
7r_Y.
=}G `i**
j(8I+||
import java.awt.Dimension; 05+uBwH
/***Example class.The x and y values should never*be negative.*/ 0k];%HV|
public class Example{ W9$mgs=S`E
private Dimension d = new Dimension (0, 0); jq4{UW'
public Example (){ } fR4O^6c:
9bDxml1
/*** Set height and width. Both height and width must be nonnegative * or an exception is thrown.*/ 'yWv @)
public synchronized void setValues (int height,int width) throws IllegalArgumentException{ Q>FuNdUk
if (height < 0 || width < 0) L'>t:^QTh
throw new IllegalArgumentException(); ]('isq,P
d.height = height; |c]Y1WwDx
d.width = width; ?2g\y@
} !7:~"kk
pFu3FUO*;
public synchronized Dimension getValues(){ Xu1tN9:oE
// Ooops! Breaks encapsulation h.\9a3B:r
return d; x{B%TM-Ey
} ">? y\#OA
} -9 AI@^q
0CYm%p8!
ye9-%~sjX
"ct_EPr`
Example类保证了它所存储的height和width值永远非负数,试图使用setValues()方法来设置负值会触发异常。不幸的是,由于getValues()返回d的引用,而不是d的拷贝,你可以编写如下的破坏性代码: ?\7" A
sf|[oD
}
r#by%P
U[S#axak
Example ex = new Example(); RyGce'
q
Dimension d = ex.getValues(); ya9V+/i7T_
d.height = -5; |!\(eLR9>
d.width = -10; ijYSYX@
27;t,Oq}
UeVRd
Z'%k`F
现在,Example对象拥有负值了!如果getValues() 的调用者永远也不设置返回的Dimension对象的width 和height值,那么仅凭测试是不可能检测到这类的错误。 X3KPN
*lN>RWbM%
C?Sy90f
o
<lS90J
不幸的是,随着时间的推移,客户代码可能会改变返回的Dimension对象的值,这个时候,追寻错误的根源是件枯燥且费时的事情,尤其是在多线程环境中。 07T"alXf:A
&oWdBna"_
N[~"X**x
D/CSR=b
更好的方式是让getValues()返回拷贝: )ow|n^D($M
T/%s7!E
6 ]@H .8+
.[-d( #l{l
public synchronized Dimension getValues(){ X9f!F2x
return new Dimension (d.x, d.y); _fH.#C
} pP&TFy#G+'
5]WpH0kzO
^G,]("di`
gE/O29Y
现在,Example对象的内部状态就安全了。调用者可以根据需要改变它所得到的拷贝的状态,但是要修改Example对象的内部状态,必须通过setValues()才可以。 +_8*;k@F'
9:bC{n
5PPV`7Xm9
@l0#C5(:
三、常见错误3#:不必要的克隆 -Fodqq@,
_u^ S[
)g9&fGYf
R4<