你的代码真的很健壮吗

上一篇 / 下一篇  2007-11-02 10:49:29 / 个人分类:你的代码要跟男人的身体一样健壮!

编写对话框程序的时候,我们时常会需要Enable或Disable某个控件,有些追求代码健壮的程序员会写出这样的代码:
s/q*V)@%L M0
Y u6w4[5|x#L0
void CMyDialog::OnStart()
dfTU1`\p-A0{
Cf#\ Jh9F0 CWnd* pBtn = GetDlgItem( IDC_ADD_BTN);
2Sn:w#_ L&G0 if( pBtn ){测试爱好者Y|!Qx9X|0j!L%s
  pBtn->EnableWindow(FALSE);测试爱好者HB'T:w[I2{_"KQqS
 }
c,b3d7wf\,y2L0 …
,N3T w z] X|2t6c0}
w2j pjC0测试爱好者tv_Q0A4li i(DM
void CMyDialog::OnAdd()
4_@N%K|X0{测试爱好者'C T8K-V3Sf
 ......测试爱好者[[2CL@7l#s(q
}

Xzg2qj*KWx Vc1^0  由于GetDlgItem()返回的是一个CWnd的指针,按照文档的描述,如果指定的控件不存在,该函数会返回一个NULL指针,为了确保不会调用NULL指针的函数,我们先检查了返回的指针是否为NULL。
6S/x9h8?C$ygO7jJ _u0
2[I o(Gb8k*E&P S0  一切看上去很美,这段程序永远不会使你的程序崩溃。然而,不会崩溃的程序,不一定是没有问题的程序。假设在MyDialog中Add按钮被定义成IDC_BTN_ADD,并且不凑巧,在这个项目的另一个Dialog里也有一个Add按钮,而且它的ID被定义为IDC_ADD_BTN,所以你的程序在编译和连结时都不会有错误。当用户使用时,也不会注意到有什么不妥,只是Dialog上的某个按钮没有变成灰色,没有人会注意到它的。测试爱好者a*VA5O]D D

$h se3V/f,C0  然而,它并不符合你的设计,也许在程序的其他地方,你假设在任务开始后,OnAdd()函数不会被调用到。这些问题一直隐藏着,直到有一天,用户报告说按Add按钮,加入某些数据后,按Ok,结果程序崩溃了。你在自己的机器上试了一下,由于之前你没有按过Start按钮,所以你一直复制不出这个问题。经过几个来回的email或者电话交流,你找到了复制错误的方法,并且奇怪为什么Add按钮没有被禁止呢,奇怪??忙活半天后,你发现原来是一个ID写错了。测试爱好者G%g9[ N[ w
测试爱好者#H f}c8[6OFh`?c
  一个很小的错误,修正它也许只要两分钟,找到它却花费了你几十分钟甚至更长。然而,这一切是可以避免的。这里我们要避免的不是说写错ID,粗心大意的错误,人人都会犯,而且会不停的犯。但是如果错误能够被及时发现,就会剩下许多时间。测试爱好者t^G&\;_u
测试爱好者$Ht+f#]Rf
  造成以上问题的原因是我们在代码中加入了一些防御性的代码,这些代码保护了程序员犯的错误。如果GetDlgItem()返回NULL,一定是由于程序员的错误。由于错误被掩盖起来,所以当问题被暴露出来时就已经面目全非了。
o3Qd"d u;u&C0
L0r/p L?4~l w0  一个比较好的做法是除去防御性代码,让问题及早暴露:
)UUO$tz Ka0
_k A;s4OI [.m0
void CMyDialog::OnStart()测试爱好者8S2}dH'I'jP1Io
{测试爱好者f.Q&o(u&D3s
GetDlgItem( IDC_ADD_BTN)->EnableWindow(FALSE);
O M)F DD j)c0
P7r7Fq st0}

KuB1p)@0  这样的结果是:一按Start按钮,程序立刻就崩溃了。的确,崩溃是很严重的错误,在Bug List里它的优先级是比较高的(仅次于造成整个OS崩溃)。但是,既然有错误,迟早要崩溃的,还不如早一点崩溃。至少早一点崩溃可以使你很快就发现问题,找到问题。有经验的程序员都清楚,一触即发的问题并不可怕,可怕的是那些偶然发生,不容易复制的问题。
j,W%Vl R0测试爱好者IBA5e{,of~A)Z
  需要在函数里检查参数的合法性吗?测试爱好者.a:ih;R o1wa5v"AZ

[&Y CaUI/PMDp0  在实现一个函数时,出于“健壮性”的考虑,我们经常会在函数的入口处加入许多参数检查代码。比如以下的一个例子:
5`'m7O$P4F&R9z4_0
&s%l*Z#wD8\K0
class CItemManager
s CuJ |5VgDZ0{
%nA'T#K4x)O3g0 protected:
:P c5m4H$P:H*x7p0  int m_nCount;测试爱好者 N&MKiGOG"MVJ
  …测试爱好者#W8?Y.Iz} _!B
 public:
`6@)}/n-ao3Y(g0  int GetItemCount();测试爱好者6J{3p4Q1M0n2]L?
  CItem* GetItem( int nIndex );测试爱好者7u/V#B.Y#DGK
};测试爱好者`]e%j'iW

4\'f`:Y;J F-S8L{0CItem* CItemManager::GetItem( int nIndex )测试爱好者2}K#b*li"WG5o
{
d1iu;y@ CBo0 if( nIndex < 0 || nIndex >= m_nCount )测试爱好者-s4V,P6@(K {O
  return NULL;
/p ]w_N0 …测试爱好者 c6P+hh+|(H,o1Zto
 return pItem;测试爱好者`(g.\ ~(Ioecc
}
!Hy1l@,W,VG0
^|,A-|M F!r"B0class CItemManager
`)~4g Bc tO0{
`g;\:m4NB6Vn0protected:测试爱好者 ?&Ms'w4{V RD
 int m_nCount;
;}-R2U"J%Anlr0 …
7z v;yh5\*Y*w8k0public:
Hmmvx5b N0 int GetItemCount();
8^_X5vm/]Y0 CItem* GetItem( int nIndex );
5ef b(k,Y#r3`A0};测试爱好者A~ ~%B f$W0[
测试爱好者)\Inp|d4G$r
CItem* CItemManager::GetItem( int nIndex )测试爱好者l4Z y7~J2x
{测试爱好者2o.~p5zV v wR
 if( nIndex < 0 || nIndex >= m_nCount )测试爱好者T \)zZ$H3DV
  return NULL;测试爱好者#rF/F s9o3LGId
 …测试爱好者gq;q \]"Jt8^K
 return pItem;测试爱好者!j8qcW%v&FA }8v~
}
测试爱好者 W4QU*k1q:p#iL/^
  在实现GetItem()时,你首先检查了参数的合法性,如果不合法就返回一个NULL指针。这样你的函数在任何的输入情况下都不会导致程序崩溃,一切看上去完美无缺,无可挑剔。但是,这样做真的能使我们的程序更健壮吗?
1{T/cH3BES,S1G0
G&zz4@ dF0  我们从调用者的角度来分析一下。为什么调用者会传入一个不合法的参数呢?一种情况是调用者的程序有bug;另一种情况是调用者不确定index是不是合法,但是他不想多写两行代码来判断index的合法性,他希望GetItem()能够一次都给办了:即能检查index的合法性,又能返回CItem的指针。
jC rtS,\.eR K0测试爱好者%Tx/m-]2f#np J
  考虑第一种情况,也许调用者写了如下的代码:测试爱好者6?dL }cP2g+IwL~:W2h

,e }1}lq0
int index;
1d7U5VdlF1eh!]$K'S0测试爱好者W*g:mG LAD0lWO

c;j5N a^ i x0p9h|Y0CItem* pItem = im.GetItem( index );
7n+pT$f!o$oz&n6v0if( pItem ){//should be executed测试爱好者I\ZB@%iU'a2`)V$f*Q

}2[!f l!m(q0}

^"a%v3c2D6f eJ,a0  这是一段危险的函数,index变量在使用之前没有初始化,但是这段程序不会,永远也不会使程序崩溃,这要感谢实现CItemManager和使用CItemManager的程序员,他们都习惯于写“健壮的”代码。但是,这段程序却不会按照我们想象的那样运行。本该执行的代码并不是每次都被执行到,因为谁也不确定index变量里存的是什么东西。这段代码是健壮的,他不会使程序崩溃,但是程序的运行过程却是不确定的。一旦出现问题,这个问题即不容易复制,也不容易确定错误原因,它的表现形式往往出乎你的意料。
oBx/r| ^$~.e0测试爱好者LC,s$U*W w
  考虑第二种情况,调用者通过其他函数得到了一个index,然后他想取得这个index下的CItem指针,但是他不想多写两行代码去判断这个index的合法性,他想:“如果这个index是合法的就请返回给我这个index下的CItem指针,如果不合法就返回一个NULL好了“。这样做只要一行代码就够了,他省下了一行代码,也许不止一行,因为在很多地方都需要呼叫GetItem()这个函数,所以,他省下了许多行代码。他会这样使用GetItem():测试爱好者Y3vD2lC~#X;T [i

Pz#AKg!LT$G0
void SomeFunc( int nIndex )
IZN`'`b0{测试爱好者"BlX#r5o{X:S!t
 CItem* pItem = im.GetItem( nIndex );测试爱好者O ZSne:@B
 if( pItem ){测试爱好者F2S(D7`:Kt(HX
  //do something.
9CY9Q C1[1ci@.Z0 }
D7g@vQLbY b0}测试爱好者!a ~}l}A^
测试爱好者r&cF#X%mmq,O
void SomeFunc( int nIndex )
8}9F ss!vRU0{
]%H"Od.QY/p b)b0 CItem* pItem = im.GetItem( nIndex );测试爱好者'a(WFM9@!S @U$|1m5lF
 if( pItem ){测试爱好者C#a5_KV r"k+U
  //do something.
;I7mrJ N0 }测试爱好者A_h,_Ja vy?]
}

2]JN G9b iJ0  如果CItemManager的实现者和使用者是同一个程序员,我们经常会写出像上面的代码,毕竟可以省下一行代码,而且上面的代码看去还不错,简洁明了。但是仔细推敲一下, 我们发现,按照以上的要求实现的GetItem()是一个不良的设计。
Z|*Q)LNk0测试爱好者8E wbjQ3v
  首先,它违反了单一职责原则(SRP)。按照以上的要求实现的GetItem()其实完成了两项功能:第一项功能用来判断index是否合法;第二项功能用来取得指定index下的CItem指针。GetItem()只应该负责取得指定index下的CItem指针,检查index的合法性应该交给其他的函数。这里,调用者可以通过GetItemCount()来判断index的合法性。测试爱好者n(Bg`I n`0y r%L"@i

5m.p| P2\Pkp,^0  其次,函数的返回值具有二义性。如果函数返回NULL,那么这个NULL可以代表index不合法,也可以解释为,指定的index下的值就是NULL,因为从编译器的角度NULL也是一个CItem指针。这里GetItem()混合了两种功能的返回值,而且第一项功能的返回值使用了第二项功能的返回值中的一个特例。这样的设计破坏了程序的完整性。假如CItemManager不是管理CItem指针,而是管理CItem对象,你就不会那样设计GetItem()函数了。测试爱好者't3F/~ AO i/klG7NE

8d|fT9N0  如果真的想在GetItem()里实现index的合法性检查,那么GetItem()的定义应该改成这样:测试爱好者1b |Gk Z2`
测试爱好者7HZ_?Z F3a
bool GetItem( int index, CItem* & pItem );
测试爱好者1g4l)F hE g
  如果index不合法,函数返回false;如果index合法,函数返回true,并且pItem返回该index下的CItem指针。经过这么一改,返回值的二义性被消除了,但是你是否觉得,GetItem()的语义已经有点变味了,这更像是在实现FindItem了。然而,按照index去Find一个Item似乎又不合理,我们进入了一个两难的境地。测试爱好者p7d:VHF!nC

#Ou\6PAM6UG,j9D b7E0  退一步海阔天空。在GetItem()里检查index的合法性,并不会让我们的程序更健壮。一个比较好的做法是,由调用者负责index的合法性检查。测试爱好者\6u8W}(s [[/_?
测试爱好者Zhb(u M ]Wk"E
  所以 SomeFunc应该改成这样:测试爱好者:xm sI;U

7~8P c {6j0
void SomeFunc( int nIndex )测试爱好者3n1FX6nE
{
w6?9\^"N-iS0 if( nIndex >= im.GetItemCount( ) )
7LR4|k;{Q:W[~.n0  return;
|MySr(a6S.e ]$k#}2w)A0 CItem* pItem = im.GetItem( nIndex );
zb{K5Dd0 pItem->…;测试爱好者G U.na,v6a2C_L
}测试爱好者)G6KI W4aHlY5Q
测试爱好者y D:z]E
void SomeFunc( int nIndex )测试爱好者oao1s W3\he
{
2aR \~/h [*N0if( nIndex >= im.GetItemCount( ) )测试爱好者(|#O r$y@gV_
return;测试爱好者a A$`{+] s.z9x_
CItem* pItem = im.GetItem( nIndex );测试爱好者Ito|+s3?6l
pItem->…;
CO-[ x&|wn9O^!Y0}
测试爱好者_ Cc3k4B {2wAR
  而GetItem()的实现应该改成这样:测试爱好者*g.@`#e*OD3|AL3?"L V
测试爱好者u {&Ok0?n
CItem* CItemManager::GetItem( int nIndex )
3d/ZXA7T^0{测试爱好者S1tBg+O%c
 ASSERT( nIndex >= 0 && nIndex < m_nCount );
&B9]IB"q5N1[0 …测试爱好者A%aY An#H0Q
 return pItem;测试爱好者L%tq1y7kMmE|%^
}
测试爱好者gI f.{v!b8k7a+T
  以上的实现我们使用了ASSERT()来检查参数的合法性,当参数不合法时,程序会被终止。ASSERT()断言只有在调试版才有效,所以程序不能依赖它来做错误处理。ASSERT()在这里的作用是,一方面在调试程序的时候,能够帮助我们尽早的发现错误。错误越早被发现,越容易被解决;另一方面,按照Robert Martin在Agile Software Development一书中所述,软件具有三项职责,最后一项便是:和阅读它的人沟通1。这些断言代码可以向阅读代码的人传递这样的信息,当程序运行到这里的时候,必须满足这些条件。
P$T{7H)WT0测试爱好者j!x0Y2a1`Q"qQ
  我是否在鼓励不要写防御性代码?
#CND\R,Qy t5U0
/H)w qO-c0n!L0  读到这里,你也许觉得我在鼓励不要在函数里检查参数的合法性,不要写防御性代码。是这样吗?答案显然是否定的。我要强调的是不要盲目的加入防御性代码,这样做并不能增强系统的健壮性。当要加入防御性代码的时候,你需要测试爱好者$p-{.c8Bz
测试爱好者$sp8r#F%c9]$Xr
  分析一下,这个条件是应该假设的,还是应该防御的。对于应该假设的条件,可以使用ASSERT()断言来检查,对于应该防御的条件,必须用专门的代码来处理。测试爱好者$DGg/V O6b o j3y ["w
测试爱好者 X~/J%b(P~k
  那么如何判断一个条件是应该假设的,还是应该防御的呢?这让我想起了荣耀先生(optimizer)的两篇沉思录,《你防御了吗?》2和《别人的棺材》3。测试爱好者j{0A$Zl;^4Y

v ij f-K:r9Oe(R)m_0  《你防御了吗?》说的是作者写的一个用于显示SQL语句的程序,作者假设输入的SQL语句不会超过4000个字符,结果有一天的确有人输入了超过4000个字符的SQL语句,然后程序崩溃了。这引起了作者对防御性编程的思考。测试爱好者z1K&yXd AC K9s?

:?.|hR(tM[U0  《别人的棺材》说的正好是一个相反的例子。有A,B,C等模块,A负责分析,执行指令,B负责生成指令。这样的设计十分合理, B不用考虑指令是否合法,由A负责指令的检查、分析,然后再执行。但是,也许负责B模块的人觉得A模块不可靠或是效率太低,所以他也加入了对指令的分析模块。作者认为这样的设计会产生冗余,当需要修改指令分析流程时,许多模块需要修改,系统变得难于维护。
7FJiq"{T0测试爱好者wj#k0p)]xGFr
  在网上的评论中,有的人认为这两篇文章相互矛盾。我觉得相反,这两篇文章恰巧揭示出需要防御和假设的两种情况。对于前一个例子,应该防御的条件,作者做了假设;对于后一个例子,B模块应该假设,却多做了一份防御。
wQ)|kB#y/|ii$i0测试爱好者D;xH dp:^7[0AP#{9\
  当我们做假设的时候,切忌不能凭空假设,我们必须清楚谁对这个假设负责。所谓对假设负责,其实就是在划分系统的职责。当我们假设一个条件时,就是把保证这个条件成立的职责分配到外部系统。在做这种划分的时候,我们应该确信外部系统有这个能力,并且这种划分是合理的。在《你防御了吗?》中,作者把保证SQL语句不会超过4000个字符的职责交给最终用户,这显然不可靠。
al)s@7l"^5aa0
3h w X3k6D*~(b0  当我们要防御一个条件时,切忌不要过度防御,过度防御虽然不会造成程序崩溃,但是会影响系统的结构。《别人的棺材》中,B模块就属于过度防御。造成过度防御的原因,我以为主要有两点:一点是由于程序员的“悲观”态度和简单分析造成的。在悲观的态度下,程序员认为一切条件都不可靠,然后,不加分析的一概做防御处理;另一点是由于社会原因造成的,我猜想《别人的棺材》中,作者就碰到了这类由于团队内部沟通上造成的。我也碰到过类似的情况,以GetItem()为例,本来我们在GetItem里是不检查index的合法性的。但是突然有一天,另一名使用这个函数的工程师告诉你,程序在GetItem里崩溃了,你调试后,告诉他,他必须负责检查index的合法性。然而,也许他是你的老板,或者你是个“菜鸟”,你争执不过他,最后只好你修改代码,加入index的检查代码,这样程序再崩溃的时候,至少不会在你写的代码里,“万事大吉”了。
-RY }/@c*H:n$PmW;S0测试爱好者Z _l5V?b
  结束
/|leo5A(u0测试爱好者6g[(d-HS](d
  当我们追求一个目标时,由于时间很长,过程艰难,到后来真正的目标往往会变得模糊不清。什么才是健壮的程序?能够正确运行的程序才是健壮的程序。在追求写出健壮的程序时,我们往往只考虑程序会不会崩溃,更有甚者,只考虑程序会不会崩溃在自己写的代码里,这离健壮程序的目标已经偏离了许多。这时有必要停下来,想一想,反思一下我们的目标和经历的过程。这篇文章就是我的一次反思。


TAG:

 

评分:0

我来说两句

显示全部

:loveliness: :handshake :victory: :funk: :time: :kiss: :call: :hug: :lol :'( :Q :L ;P :$ :P :o :@ :D :( :)

Open Toolbar