|
摘要: 本文通过简单通俗的例子告诉我们如何判断代码的稳定性和代码中的异类并且如何重构此类代码 异味这个词可能有点抽象我们先看一下下面的例子 这是一个CAD系统现在它已经可以画三种形状了:线条长方形跟圆先认真的看一下下面的代码: classShape{ finalstaticintTYPELINE=; finalstaticintTYPERECTANGLE=; finalstaticintTYPECIRCLE=; intshapeType; //线条的开始点 //长方形左下角的点 //圆心 Pointp; //线条的结束点 //长方形的右上角的点 //如果是圆的话这个属性不用 Pointp; intradius; } classCADApp{ voiddrawShapes(GraphicsgraphicsShapeshapes[]){ for(inti=;i<shapeslength;i++){ switch(shapes[i]getType()){ caseShapeTYPELINE: graphicsdrawLine(shapes[i]getP()shapes[i]getP()); break; caseShapeTYPERECTANGLE: //画四条边 graphicsdrawLine(); graphicsdrawLine(); graphicsdrawLine(); graphicsdrawLine(); break; caseShapeTYPECIRCLE: graphicsdrawCircle(shapes[i]getP()shapes[i]getRadius()); break; } } } } 代码都是一直在改变的而这也是上面的代码会碰到的一个问题 现在我们有一个问题:如果我们需要支持更多的形状(比如三角形)那么肯定要改动Shape这个类CADApp里面的drawShapes这个方法也要改 好改为如下的样子: classShape{ finalstaticintTYPELINE=; finalstaticintTYPERECTANGLE=; finalstaticintTYPECIRCLE=; finalstaticintTYPETRIANGLE=; intshapeType; Pointp; Pointp; //三角形的第三个点 Pointp; intradius; } classCADApp{ voiddrawShapes(GraphicsgraphicsShapeshapes[]){ for(inti=;i<shapeslength;i++){ switch(shapes[i]getType()){ caseShapeTYPELINE: graphicsdrawLine(shapes[i]getP()shapes[i]getP()); break; caseShapeTYPERECTANGLE: //画四条边 graphicsdrawLine(); graphicsdrawLine(); graphicsdrawLine(); graphicsdrawLine(); break; caseShapeTYPECIRCLE: graphicsdrawCircle(shapes[i]getP()shapes[i]getRadius()); break; caseShapeTYPETRIANGLE: graphicsdrawLine(shapes[i]getP()shapes[i]getP()); graphicsdrawLine(shapes[i]getP()shapes[i]getP()); graphicsdrawLine(shapes[i]getP()shapes[i]getP()); break; } } } } 如果以后要支持更多的形状这些类又要改动……这可不是什么好事情！ 理想情况下我们希望当一个类一个方法或其他的代码设计完以后就不用再做修改了它们应该稳定到不用修改就可以重用 现在的情况恰好相反！ 每当我们增加新的形状都得修改Shape这个类跟CADApp里面的drawShapes方法 怎么让代码稳定（也就是无需修改）？这个问题是个好问题！不过老规矩先不说我们以行动回答 我们先看看另外一个方法当给你一段代码你怎么知道它是稳定的？
怎么判断代码的稳定性？ 要判断代码的稳定性我们可能会这样来判定先假设一些具体的情况或者需求变动了然后来看一看要满足这些新的需求代码是否需要被修改？ 可惜这也是一件很麻烦的事因为有那么多的可能性！我们怎么知道哪个可能性要考虑哪些不用考虑？ 有个更简单的方法如果发现说我们已经第三次修改这些代码了那我们就认定这些代码是不稳定的这个方法很懒惰而且被动！我们被伤到了才开始处理状况不过至少这种方法还是一个很有效的方法 此外还有一个简单而且主动的方法如果这段代码是不稳定或者有一些潜在问题的那么代码往往会包含一些明显的痕迹正如食物要腐坏之前经常会发出一些异味一样（当然食物如果有异味了再怎么处理我们都不想吃了但是代码可不行）我们管这些痕迹叫做代码异味正如并不是所有的食物有异味都不能吃了但大多数情况下确实是不能吃了并不是所有的代码异味都是坏事但大多数情况下它们确实是坏事情！因此当我们感觉出有代码异味时我们必须小心谨慎的检查了 现在我们来看看上面例子中的代码异味吧 示例代码中的代码异味 第一种异味代码用了类别代码（typecode） classShape{ finalintTYPELINE=; finalintTYPERECTANGLE=; finalintTYPECIRCLE=; intshapeType;
} 这样的异味是一种严肃的警告我们的代码可能有许多问题 第二种异味Shape这个类有很多属性有时候是不用的例如radius这个属性只有在这个Shape是个圆的时候才用到 classShape{
Pointp; Pointp; intradius;//有时候不用 } 第三种异味我们想给pp取个好一点的变量名都做不到因为不同的情况下它们有不同的含义 classShape{
Pointp;//要取作起始点左下点还是圆心？ Pointp; } 第四种异味drawShapes这个方法里面有个switch表达式当我们用到switch（或者一大串的ifthenelseif）时小心了switch表达式经常是跟类别代码（typecode）同时出现的 现在让我们将这个示例中的代码异味消除吧 消除代码异味怎么去掉类别代码（typecode） 大多数情况下要想去掉一个类别代码我们会为每一种类别建立一个子类比如 （当然并不是每次要去掉一个类别代码都要增加一个新类我们下面的另一个例子里面会讲另一种解决方法） classShape{ } classLineextendsShape{ PointstartPoint; PointendPoint; } classRectangleextendsShape{ PointlowerLeftCorner; PointupperRightCorner; } classCircleextendsShape{ Pointcenter; intradius; } 因为现在没有类别代码了drawShapes这个方法里面就要用instanceof来判断对象是哪一种形状了因此我们不能用switch了而要改用ifthenelse classCADApp{ voiddrawShapes(GraphicsgraphicsShapeshapes[]){ for(inti=;i<shapeslength;i++){ if(shapes[i]instanceofLine){ Lineline=(Line)shapes[i]; graphicsdrawLine(linegetStartPoint()linegetEndPoint()); }elseif(shapes[i]instanceofRectangle){ Rectanglerect=(Rectangle)shapes[i]; graphicsdrawLine(); graphicsdrawLine(); graphicsdrawLine(); graphicsdrawLine(); }elseif(shapes[i]instanceofCircle){ Circlecircle=(Circle)shapes[i]; graphicsdrawCircle(circlegetCenter()circlegetRadius()); } } } } 因为没有类别代码了现在每个类（ShapeLineRectangleCircle）里面的所有属性就不会有时用得到有时用不到了现在我们也可以给它们取一些好听点的名字了（比如在Line里面p这个属性可以改名为startPoint了）现在四种异味只剩一种了那就是在drawShapes里面还是有一大串ifthenelseif我们下一步就是要去掉这长长的一串 消除代码异味如何去掉一大串ifthenelseif（或者switch） 经常地为了去掉ifthenelseif或者switch我们需要先保证在每个条件分支下的要写的代码是一样的在drawShapes这个方法里面我们先以一个较抽象的方法（伪码）来写吧 classCADApp{ voiddrawShapes(GraphicsgraphicsShapeshapes[]){ for(inti=;i<shapeslength;i++){ if(shapes[i]instanceofLine){ 画线条; }elseif(shapes[i]instanceofRectangle){ 画长方形; }elseif(shapes[i]instanceofCircle){ 画圆; } } } } 条件下的代码还是不怎么一样不如再抽象一点 classCADApp{ voiddrawShapes(GraphicsgraphicsShapeshapes[]){ for(inti=;i<shapeslength;i++){ if(shapes[i]instanceofLine){ 画出形状; }elseif(shapes[i]instanceofRectangle){ 画出形状; }elseif(shapes[i]instanceofCircle){ 画出形状; } } } } 好现在三个分支下的代码都一样了我们也就不需要条件分支了 classCADApp{ voiddrawShapes(GraphicsgraphicsShapeshapes[]){ for(inti=;i<shapeslength;i++){ 画出形状; } } } 最后将画出形状这个伪码写成代码吧: classCADApp{ voiddrawShapes(GraphicsgraphicsShapeshapes[]){ for(inti=;i<shapeslength;i++){ shapes[i]draw(graphics); } } } 当然我们需要在每种Shape的类里面提供draw这个方法 abstractclassShape{ abstractvoiddraw(Graphicsgraphics); } classLineextendsShape{ PointstartPoint; PointendPoint; voiddraw(Graphicsgraphics){ graphicsdrawLine(getStartPoint()getEndPoint()); } } classRectangleextendsShape{ PointlowerLeftCorner; PointupperRightCorner; voiddraw(Graphicsgraphics){ graphicsdrawLine(); graphicsdrawLine(); graphicsdrawLine(); graphicsdrawLine(); } } classCircleextendsShape{ Pointcenter; intradius; voiddraw(Graphicsgraphics){ graphicsdrawCircle(getCenter()getRadius()); } } 将抽象类变成接口 现在看一下Shape这个类它本身没有实际的方法所以它更应该是一个接口 interfaceShape{ voiddraw(Graphicsgraphics); } classLineimplementsShape{
} classRectangleimplementsShape{
} classCircleimplementsShape{
} 改进后的代码 改进后的代码就像下面这样 interfaceShape{ voiddraw(Graphicsgraphics); } classLineimplementsShape{ PointstartPoint; PointendPoint; voiddraw(Graphicsgraphics){ graphicsdrawLine(getStartPoint()getEndPoint()); } } classRectangleimplementsShape{ PointlowerLeftCorner; PointupperRightCorner; voiddraw(Graphicsgraphics){ graphicsdrawLine(); graphicsdrawLine(); graphicsdrawLine(); graphicsdrawLine(); } } classCircleimplementsShape{ Pointcenter; intradius; voiddraw(Graphicsgraphics){ graphicsdrawCircle(getCenter()getRadius()); } } classCADApp{ voiddrawShapes(GraphicsgraphicsShapeshapes[]){ for(inti=;i<shapeslength;i++){ shapes[i]draw(graphics); } } } 如果我们想要支持更多的图形（比如三角形）上面没有一个类需要修改我们只需要创建一个新的类Triangle就行了 另一个例子 让我们来看一下另外一个例子在当前的系统中有三种用户常规用户管理员和游客 常规用户必须每隔天修改一次密码（更频繁也行）管理员必须每天修改一次密码游客就不需要修改了 常规用户跟管理员可以打印报表 先看一下当前的代码 classUserAccount{ finalstaticintUSERTYPE_NORMAL=; finalstaticintUSERTYPE_ADMIN=; finalstaticintUSERTYPE_GUEST=; intuserType; Stringid; Stringname; Stringpassword; DatedateOfLastPasswdChange; publicbooleancheckPassword(Stringpassword){
} } classInventoryApp{ voidlogin(UserAccountuserLoggingInStringpassword){ if(userLoggingIncheckPassword(password)){ GregorianCalendartoday=newGregorianCalendar(); GregorianCalendarexpiryDate=getAccountExpiryDate(userLoggingIn); if(todayafter(expiryDate)){ //提示用户修改密码
} } } GregorianCalendargetAccountExpiryDate(UserAccountaccount){ intpasswordMaxAgeInDays=getPasswordMaxAgeInDays(account); GregorianCalendarexpiryDate=newGregorianCalendar(); expiryDatesetTime(accountdateOfLastPasswdChange); expiryDateadd(CalendarDAY_OF_MONTHpasswordMaxAgeInDays); returnexpiryDate; } intgetPasswordMaxAgeInDays(UserAccountaccount){ switch(accountgetType()){ caseUserAccountUSERTYPE_NORMAL: return; caseUserAccountUSERTYPE_ADMIN: return; caseUserAccountUSERTYPE_GUEST: returnIntegerMAX_VALUE; } } voidprintReport(UserAccountcurrentUser){ booleancanPrint; switch(currentUsergetType()){ caseUserAccountUSERTYPE_NORMAL: canPrint=true; break; caseUserAccountUSERTYPE_ADMIN: canPrint=true; break; caseUserAccountUSERTYPE_GUEST: canPrint=false; } if(!canPrint){ thrownewSecurityException(Youhavenoright); } //打印报表 } } 用一个对象代替一种类别（注意之前是一个类代替一种类别） 根据之前讲的解决方法要去掉类别代码我们只需要为每种类别创建一个子类比如 abstractclassUserAccount{ Stringid; Stringname; Stringpassword; DatedateOfLastPasswdChange; abstractintgetPasswordMaxAgeInDays(); abstractbooleancanPrintReport(); } classNormalUserAccountextendsUserAccount{ intgetPasswordMaxAgeInDays(){ return; } booleancanPrintReport(){ returntrue; } } classAdminUserAccountextendsUserAccount{ intgetPasswordMaxAgeInDays(){ return; } booleancanPrintReport(){ returntrue; } } classGuestUserAccountextendsUserAccount{ intgetPasswordMaxAgeInDays(){ returnIntegerMAX_VALUE; } booleancanPrintReport(){ returnfalse; } } 但问题是三种子类的行为（里面的代码）都差不多一样getPasswordMaxAgeInDays这个方法就一个数值不同（或者IntegerMAX_VALUE)canPrintReport这个方法也不同在一个数值（true或false）这三种用户类型只需要用三个对象代替就行了无须特地新建三个子类了 classUserAccount{ UserTypeuserType; Stringid; Stringname; Stringpassword; DatedateOfLastPasswdChange; UserTypegetType(){ returnuserType; } } classUserType{ intpasswordMaxAgeInDays; booleanallowedToPrintReport; UserType(intpasswordMaxAgeInDaysbooleanallowedToPrintReport){ thispasswordMaxAgeInDays=passwordMaxAgeInDays; thisallowedToPrintReport=allowedToPrintReport; } intgetPasswordMaxAgeInDays(){ returnpasswordMaxAgeInDays; } booleancanPrintReport(){ returnallowedToPrintReport; } staticUserTypenormalUserType=newUserType(true); staticUserTypeadminUserType=newUserType(true); staticUserTypeguestUserType=newUserType(IntegerMAX_VALUEfalse); } classInventoryApp{ voidlogin(UserAccountuserLoggingInStringpassword){ if(userLoggingIncheckPassword(password)){ GregorianCalendartoday=newGregorianCalendar(); GregorianCalendarexpiryDate=getAccountExpiryDate(userLoggingIn); if(todayafter(expiryDate)){ //提示用户修改密码
} } } GregorianCalendargetAccountExpiryDate(UserAccountaccount){ intpasswordMaxAgeInDays=getPasswordMaxAgeInDays(account); GregorianCalendarexpiryDate=newGregorianCalendar(); expiryDatesetTime(accountdateOfLastPasswdChange); expiryDateadd(CalendarDAY_OF_MONTHpasswordMaxAgeInDays); returnexpiryDate; } intgetPasswordMaxAgeInDays(UserAccountaccount){ returnaccountgetType()getPasswordMaxAgeInDays(); } voidprintReport(UserAccountcurrentUser){ booleancanPrint; canPrint=currentUsergetType()canPrintReport(); if(!canPrint){ thrownewSecurityException(Youhavenoright); } //打印报表 } } 注意到了吧用一个对象代替类别同样可以移除switch或者ifthenelseif 总结一下类别代码的移除 要移动一些类别代码和switch表达式有两种方法 用基于同一父类的不同子类来代替不同的类别 用一个类的不同对象来代替不同的类别 当不同的类别具有比较多不同的行为时用第一种方法当这些类别的行为非常相似或者只是差别在一些值上面的时候用第二个方法 普遍的代码异味 类别代码和switch表达式是比较普遍的代码异味此外还有其他的代码异味也很普遍 下面是大概的异味列表 代码重复 太多的注释 类别代码（typecode） switch或者一大串ifthenelseif 想给一个变量方法或者类名取个好名字时也怎么也取不好 用类似XXXUtilXXXManagerXXXController和其他的一些命名 在变量方法或类名中使用这些单词AndOr等等 一些实例中的变量有时有用有时没用 一个方法的代码太多或者说方法太长 一个类的代码太多或者说类太长 一个方法有太多参数 两个类都引用了彼此（依赖于彼此） |
