代码评审三步走-2

#x\k8Ti G
{%T#J0|2})o0　　第二个参数mnsInfos在整个函数中很多地方都有使用，因此要详细分析。51Testing软件测试网do-w"px I

#REL
ZSw9n0　　第一处用到是在初始化语句中：ivmnsInfos.length(mnsInfos.length());51Testing软件测试网oIj$pDZ*k

　　我们说过对输入参数仅用于外部调用的情况可以不关注，因此这句略过。51Testing软件测试网Z7R!T5J#@G S8P

;W+}#s~+C&[F~0　　第二处是作为循环终了判断条件出现：

c-M9S&~3neB051Testing软件测试网"h'Hf+}%dE|W

　　这里提出第二条检视规则：输入参数参与循环判定或条件判定时，需对该参数取值进行分析，检查程序是否存在条件遗漏。51Testing软件测试网*\eM-Tq

　　这里的取值情况比较明显，只需检查当mnsInfos.length()取值为0时程序是否有正常处理即可。从代码看出，取值为0时，跳过整个循环，输出参数（序列）长度都置为0，如果这样的处理符合设计模型，就没有问题。51Testing软件测试网f:Mf*r!^wp

*N-_?T E I2xJgU0　　请注意斜体字部分，对代码实现是否有问题不应简单、孤立地判定，而应结合设计与需求进行检查，切记。51Testing软件测试网.\ RQ:yE `6`$I,f5j

　　接下来是一个赋值语句，将mnsInfos[i].monsterInfos赋值给局部变量mnsStatusSeq。

　　这里提出第三条检视规则：变量以某种方式与输入参数建立关联的，等同于输入参数对待。

4J'~ gi7{I0　　这条规则的意思就是，变量与输入参数关联后，就同样适用于第一、第二两条规则。这里有点递归的意思。51Testing软件测试网.w)ez/Kk@5\f

　　因此，对mnsStatusSeq的后续使用，同样要与输入参数mnsInfos一样进行分析，由于分析原则一样，就不作重复说明。51Testing软件测试网[/U"M m5m8lr

7AP}5e$O*w;[0　　另外需要指出的是，在分析hrdId时，实际上有一个变量mnsTypes与hrdId建立了关联——尽管不是赋值，但是通过函数调用产生了关联（mnsTypes是函数getMonsterType的出参），因此对mnsTypes变量同样要进行相应的分析。51Testing软件测试网^1fN1l:c/I]&Sk

fw'No7go0　　在此之后直接出现mnsInfos的地方全都是赋值，同样遵循规则三处理。

　　对全部的（2个）直接输入参数分析完成后，就可以递归地对所有的“间接输入参数”（即前面提及的，通过某种方式与输入参数建立关联的局部变量，如mnsStatusSeq、mnsTypes等）进行分析。51Testing软件测试网a"gp ["b4y(V(E

`.{i"_|s/M0　　所有输入参数分析完毕后，第二步就算是完成了。

　　第三步：

　　接下来需要覆盖的，就是真正的“祸患之源”，即“非常规输入”，或者叫“隐性输入”。

ACnQ Cy0　　我们来看第二段代码：51Testing软件测试网9K|U-ih

A8JG.{6v%Ei(S0　　示例代码段二51Testing软件测试网C `yY\W

51Testing软件测试网#xRq2P)i!Y

l4O?)h!}kZnw0　　在以上的代码段里，你能找到哪些“隐性输入”？51Testing软件测试网|+_:peC4bB7JS%e

5X7]
c"k%ZH7j(I0　　我个人将“隐性输入”划分为以下四个层次：

{[B9I
egNx2e0　　层次一，我们可以很容易找到：questIDs_.length()、quest的取值、notExistQuests.find()的返回值 和stateMgr. getQuest ()的返回值。从宏观上说，凡是来自于本程序段之外的数据都应视为本程序段的输入，在进行程序走向和处理分析时，都需要予以关注。51Testing软件测试网/?[ |&s
z h|6q~E$b3z

　　如上所述的四个数值都属于该代码段的“非常规输入”，因此对它们可能的取值，都应该分别分析是否影响程序走向，是否有正确的处理。51Testing软件测试网4gS^#]ak#R'T

'Rut7|Hc(pti o0　　questIDs_.length()的处理可以参考常规输入的处理。

%n2{}M)S6~"cs)}@C!}0　　questObj的取值则需要参考cvtQuestById函数的定义（或向该函数的编写者询问相应信息），要考虑有效Id和无效Id的情况（即对象找不到）。

2T1uAw
Ng0　　同样notExistQuests.find()也会返回两种结果。51Testing软件测试网;i)tM8EpmNwV

　　而getQuest ()的结果要更复杂一些，包括正常的对象列表，空对象列表（没找到）和异常三种（即输入参数无效，questObj的取值就是无效对象）。当然，在处理输 入的questObj无效时，getQuest ()的返回可能是空对象列表，也可能是抛出异常，具体情况还是要再对getQuest ()进行分析才能确定。51Testing软件测试网8m O,F!pN!}#oVK

　　特别地，对于调用外部函数获取的输入值，如果代码编写者自己并不清楚其可能的取值范围，就需要从设计文档或该外部函数设计者处获取信息

　　层次二，还有其他的“非常规输入”吗？结合对“输入”的定义，我们能够发现cancel_也是个输入值。尽管它是当前类定义的成员变量，对 “类”这个单元来说是属于内部数据，但是对当前被检视的run函数而言还是个“外来户”。当然，根据这个标准，那些“下划线一族”也都是“外来户”，包括 questIDs_、owner_。51Testing软件测试网V*~6gd:MH]M+tw

HH&m.OSU6Z0　　如果了解函数所在类的调用上下文，可以看出run函数是以线程方式运行的，cancel_的取值在该函数运行过程中是可以改变的——相比于函数启动就固定的输入值而言，在过程中可变的输入值，更值得注意。51Testing软件测试网$St&qOryj

　　关注被检查对象运行过程中输入值的变化，是多线程模式下进行代码检视和单元测试的重点内容。

tFd:g1Gs0　　在这段代码中，cancel_的取值尽管只有true和false两种，但是结合运行中的动态变化，就可以看出能够影响程序走向和处理的有四种 状态：初始值为false，且全程不变；初始值为true，且全程不变；初始值为false，且中段变为true；初始值为true，且中段变为 false。

Z4q{
WJ,[0　　最后一种状态对程序走向没有影响，而且结合程序其他代码段来看，也没有提供从true->false的转换，因此归为无效状态，不用测试/检查。51Testing软件测试网GT6io6l#@wa%g

GY1{DqLw0　　前两种状态属于静态输入，与“常规输入”进行同样检查即可。51Testing软件测试网0N~6w}x5dG5k3WP&I4a!c

O;QsJIL&{8T+E*B0　　而第三种状态属于“动态输入”，需要检查在输入发生变化时，程序是否能够按照预期进行处理。例如在cancel后是否能够正常销毁对象，是否能 够正常处理对象队列而不出现越界现象..在这段简单的示例代码中没有出现这些问题，但稍复杂一些的代码就很有可能出现以上疏漏。

　　另外，在“层次一”中提到的questIDs_.length()其实与cancel_是同一性质的输入。从理论上说，它在循环中也是可以变化 的——譬如在某次进入循环体后，由于外部动作导致questIDs_中的部分内容被删除，此时的questIDs_及其length()都会发生变化。因 此questIDs_[processPos]这个访问就很可能会引发一个越界异常。

j2_(|2y:f3T^ j0　　当然，以上情况是基于questIDs_成员变量可以改变而言的，结合整个类的完整代码看，该变量是对象初始化时生成的，且在对象生命周期内没有提供改变的接口，因此不会发生以上异常情况。

　　第三个层次，是识别所谓的“共享资源”，这点在以上示例代码中没有直接体现。51Testing软件测试网6aA,C7StCp"O

IktCs!V0　　所谓“共享资源”，指的是部分或全部代码共同分享的资源，包括但不限于全局变量（对象）、数据库连接、数据表内容、文件、内存块、线程锁或信号 量..等等。当代码中出现数据库读写、文件读写等内容时，实际上都是对外部的“共享资源”发生了依赖。这些“共享资源”的取值范围或状态，就可能是代码中 需要考虑处理的。同样的情况也发生在并发程序获取锁或信号量时，程序代码如果没有充分考虑到会影响锁或信号量的其他代码，就很可能会导致死锁的发生。51Testing软件测试网?9?-TP3LEfgz

g'j
JFPb'Q}0在检视涉及“共享资源”的代码时，除了检查代码对这些“共享资源”的取值是否有合适处理外，还要进一步检查当前检视代码与其他相关代码是否对“共享资源”的竞争进行正确处理，它们对“共享资源”的依赖，是否构成“依赖环”。

　　要将所有的共享依赖都梳理出来不是件简单的事，篇幅所限，这里就不展开讨论了。51Testing软件测试网"~m }%ND@.l K.s9? w

　　第四个层次，是对程序中所有异常也作为“非常规输入”处理。

f4z x;\;e^E0　　异常，即Exception，是程序在遭遇非预期或不能处理的状况时，用于通知外层程序或调用者出现状况的一种机制。51Testing软件测试网:b2g+P@BjDrN

　　从最简单的a=b/c可能发生的除零异常，到复杂的函数调用可以抛出多种类型的异常，这些都是程序中常见的情形。51Testing软件测试网6`&c(g x%[:W_-}Nz

JF@;C(_8V|0　　在调用函数或进行数据处理时发生的异常，都是由外部传入/返回给当前函数（被检视函数）的，因此对于当前函数而言，异常Exception自然也应当作为一种“非常规输入”进行处理。

　　由于异常在代码段中通常是以“隐身”的方式存在着，除了函数声明时可能有的throw指示（有时候也没有）之外，难以寻觅它的踪影，因而它也是最容易被遗漏的“非常规输入”，应当作为一个重点的代码检视对象。51Testing软件测试网PO3N-T+K8J

3pY$B.r ~9fJL0　　在示例代码段二中，对cvtQuestById、appendQuestState、getQuest等方法的调用，都有可能遭遇它们抛出的异常。而从代码中也可以看到，该段程序对可能遭遇的异常确实进行了处理。51Testing软件测试网(PH[[NW

　　再看示例代码段一，它的逻辑简单，看起来信心满满，没有任何异常保护。51Testing软件测试网;_VK$D9j

N#N/?G8a7x0　　但是它真的不会遭遇任何异常吗？仔细检查可以看到其中有这样一条语句：

|V| n c@0　　NPCManagement::MonsterLoader::getMonsterType(hrdId, -1, monsterIds, mnsTypes);51Testing软件测试网I'^*k^]q

　　循着这条语句顺藤摸瓜，会看到getMonsterType方法是通过数据库查询实现的，且此处的数据库查询是通过新建数据库连接进行的。51Testing软件测试网{W,h*s,?*[n

fBgZ6J!\0W%r0　　在其中建立数据库连接的部分（以一个DBConnection构造函数的面目出现），明明白白地写出了有可能抛出各种类型的异常。换言之，当无法获取数据库连接时，getMonsterType就会抛出异常，而示例代码段一中没有捕获该异常。51Testing软件测试网6[5M"V:K9STO

%aE)i7D1@O]3fi0　　这个未捕获的异常是否会引发未定义的问题，就完全要看调用该方法的外层调用者的处理了——生存还是死亡，这是一个问题。51Testing软件测试网2n8G4\;Z"q^e;lA}

　　即便现存的所有调用者都注意到这点并进行了异常保护，也无法保证今后新增加的调用者也都会遵循这个“传统”。最好的措施还是自我保护。

　　在进行代码检视时，对调用的外部函数、系统API等，都要仔细检查其是否可能抛出异常，并检查程序代码中对可能发生的异常，是否按照规范进行了适当处理。

LOL3xxMT \0　　对异常的处理分为两个阶段：捕获与处置。按捕获情况可分为三类：捕获特定异常，捕获所有异常，不捕获异常。 捕获异常后的处置方法可以分为两类：1、记录而不处理（吞噬异常），程序继续流转；2、记录并中断程序运行，重新抛出异常。无论是哪一种捕获和处置方法， 是否“正确”都要看是否符合业务逻辑，是否符合设计规范，不能一概而论。51Testing软件测试网.w uA,Y$j

iu'T!ZN#p/p0　　一般而言，对可能存在的异常不捕获是比较危险的。因为除非明确知道外围程序有异常保护机制，否则本程序对异常不处理会导致异常扩散，很有可能导致产品崩溃、错乱，发生无法预知的后果。51Testing软件测试网&JY2k?b5r

csbk/g.zV9L!n0　　以上就是我所提倡的“代码检视三步走”的全部内容。受个人能力和当前工作重点所限，以上给出的内容既不全面也可能存在许多错漏之处。希望能够起到抛砖引玉的作用，引出更多专家的宝贵经验。51Testing软件测试网2g/X]6m@