敏捷开发技巧-消除代码异味

来源:互联网 发布:网络红猫红小胖 编辑:程序博客网 时间:2024/03/29 09:49
本文通过简单通俗的例子, 告诉我们如何判断代码的稳定性和代码中的异类, 并且如何重构此类代码.

敏捷开发技巧-消除代码异味

Wingel 发表于 2006-12-13 08:41:41
作者:Wingel     来源:Wingel's blog

摘要:

本文通过简单通俗的例子, 告诉我们如何判断代码的稳定性和代码中的异类, 并且如何重构此类代码.


 

异味这个词,可能有点抽象,我们先看一下下面的例子

这是一个CAD系统. 现在,它已经可以画三种形状了:线条,长方形,跟圆.先认真的看一下下面的代码:

class Shape {                                                                                   
       final static int TYPELINE = 0;                                                              
       final static int TYPERECTANGLE = 1;                                                          
       final static int TYPECIRCLE = 2;                                                            
       int shapeType;                                                                              
       //线条的开始点
     //长方形左下角的点
     //圆心
     Point p1;                                                                                    
       //线条的结束点
     //长方形的右上角的点
     //如果是圆的话,这个属性不用
     Point p2;                                                                                    
       int radius;                                                                                  
    }                                                                                              
class CADApp {                                                                                  
       void drawShapes(Graphics graphics, Shape shapes[]) {                                        
           for (int i = 0; i < shapes.length; i++) {                                                
               switch (shapes[i].getType()) {                                                      
                  case Shape.TYPELINE:                                                              
                      graphics.drawLine(shapes[i].getP1(), shapes[i].getP2());                      
                      break;                                                                        
                  case Shape.TYPERECTANGLE:                                                        
                      //画四条边
               graphics.drawLine(...);                                                      
                      graphics.drawLine(...);                                                      
                      graphics.drawLine(...);                                                      
                      graphics.drawLine(...);                                                      
                      break;                                                                        
                  case Shape.TYPECIRCLE:                                                            
                      graphics.drawCircle(shapes[i].getP1(), shapes[i].getRadius());                
                      break;                                                                        
               }                                                                                    
           }                                                                                        
       }                                                                                            
    }
                                                                                              


    代码都是一直在改变的,而这也是上面的代码会碰到的一个问题.

    现在我们有一个问题: 如果我们需要支持更多的形状(比如三角形), 那么肯定要改动Shape这个类, CADApp里面的drawShapes这个方法也要改.
好,改为如下的样子:
    
class Shape {      
       final static int TYPELINE = 0;
       final static int TYPERECTANGLE = 1;
       final static int TYPECIRCLE = 2;
       final static int TYPETRIANGLE = 3;
       int shapeType;  
       Point p1;      
       Point p2;      
       //三角形的第三个点.
       Point p3;      
       int radius;    
    }                  
class CADApp {    
     void drawShapes(Graphics graphics, Shape shapes[]) {
         for (int i = 0; i < shapes.length; i++) {
             switch (shapes[i].getType()) {
                case Shape.TYPELINE:
                    graphics.drawLine(shapes[i].getP1(), shapes[i].getP2());
                    break;
                case Shape.TYPERECTANGLE:
                      //画四条边.
                    graphics.drawLine(...);
                    graphics.drawLine(...);
                    graphics.drawLine(...);
                    graphics.drawLine(...);
                    break;
                case Shape.TYPECIRCLE:
                    graphics.drawCircle(shapes[i].getP1(), shapes[i].getRadius());
                    break;
                case Shape.TYPETRIANGLE:
                    graphics.drawLine(shapes[i].getP1(), shapes[i].getP2());
                    graphics.drawLine(shapes[i].getP2(), shapes[i].getP3());
                    graphics.drawLine(shapes[i].getP3(), shapes[i].getP1());
                    break;
               }      
           }          
       }              
  }
                  

    如果以后要支持更多的形状,这些类又要改动……,这可不是什么好事情!
   理想情况下,我们希望当一个类,一个方法或其他的代码设计完以后,就不用再做修改了。它们应该稳定到不用修改就可以重用。
    现在的情况恰好相反!
    每当我们增加新的形状,都得修改Shape这个类,跟CADApp里面的drawShapes方法。

    怎么让代码稳定(也就是无需修改)?这个问题是个好问题!不过老规矩,先不说,我们以行动回答。
    我们先看看另外一个方法: 当给你一段代码,你怎么知道它是稳定的?


    怎么判断代码的稳定性?

    要判断代码的稳定性,我们可能会这样来判定:先假设一些具体的情况或者需求变动了,然后来看一看,要满足这些新的需求,代码是否需要被修改?
    可惜,这也是一件很麻烦的事,因为有那么多的可能性!我们怎么知道哪个可能性要考虑,哪些不用考虑?

    有个更简单的方法, 如果发现说,我们已经第三次修改这些代码了,那我们就认定这些代码是不稳定的。这个方法很“懒惰”,而且“被动”!我们被伤到了,才开始处理状况。不过至少这种方法还是一个很有效的方法。

    此外,还有一个简单,而且“主动”的方法:如果这段代码是不稳定或者有一些潜在问题的,那么代码往往会包含一些明显的痕迹。正如食物要腐坏之前,经常会发出一些异味一样(当然,食物如果有异味了,再怎么处理我们都不想吃了。但是代码可不行。)。我们管这些痕迹叫做“代码异味”。正如并不是所有的食物有异味都不能吃了,但大多数情况下,确实是不能吃了。并不是所有的代码异味都是坏事,但大多数情况下,它们确实是坏事情!因此,当我们感觉出有代码异味时,我们必须小心谨慎的检查了。

    现在,我们来看看上面例子中的代码异味吧。

    示例代码中的代码异味:

    第一种异味:代码用了类别代码(type code)。
  
class Shape {                                                                                  
     final int TYPELINE = 0;                                                                      
     final int TYPERECTANGLE = 1;                                                                
     final int TYPECIRCLE = 2;                                                                    
     int shapeType;                                                                              
     ...                                                                                          
  }    
                                                                                          

    这样的异味,是一种严肃的警告:我们的代码可能有许多问题。      

    第二种异味:Shape这个类有很多属性有时候是不用的。例如,radius这个属性只有在这个Shape是个圆的时候才用到:

    class Shape {                                                                                   
       ...                                                                                          
       Point p1;                                                                                    
       Point p2;                                                                                    
       int radius; //有时候不用                                                            
    }  
                                                                                            

    第三种异味:我们想给p1,p2取个好一点的变量名都做不到,因为不同的情况下,它们有不同的含义:

    class Shape {  
       ...        
       Point p1; //要取作“起始点”,“左下点”,还是“圆心”?
     Point p2;  
    }        
      

    第四种异味:drawShapes这个方法里面,有个switch表达式。当我们用到switch(或者一大串的if-then-else-if)时,小心了。switch表达式经常是跟类别代码(type code)同时出现的。
    现在,让我们将这个示例中的代码异味消除吧。

    消除代码异味:怎么去掉类别代码(type code)

    大多数情况下,要想去掉一个类别代码,我们会为每一种类别建立一个子类,比如:
    (当然,并不是每次要去掉一个类别代码都要增加一个新类,我们下面的另一个例子里面会讲另一种解决方法)

    class Shape {  
    }              
    class Line extends Shape {
       Point startPoint;
       Point endPoint;
    }              
    class Rectangle extends Shape {
       Point lowerLeftCorner;
       Point upperRightCorner;
    }              
    class Circle extends Shape {
       Point center;
       int radius;
    }              


    因为现在没有类别代码了,drawShapes这个方法里面,就要用instanceof来判断对象是哪一种形状了。因此,我们不能用switch了,而要改用if-then-else:
    class CADApp { 
       void drawShapes(Graphics graphics, Shape shapes[]) {
           for (int i = 0; i < shapes.length; i++) {
               if (shapes[i] instanceof Line) {
                  Line line = (Line)shapes[i];
                  graphics.drawLine(line.getStartPoint(),line.getEndPoint());
               } else if (shapes[i] instanceof Rectangle) {
                  Rectangle rect = (Rectangle)shapes[i];
                  graphics.drawLine(...);
                  graphics.drawLine(...);
                  graphics.drawLine(...);
                  graphics.drawLine(...);
               } else if (shapes[i] instanceof Circle) {
                  Circle circle = (Circle)shapes[i];
                  graphics.drawCircle(circle.getCenter(), circle.getRadius());                      
               }                                                                                    
           }                                                                                        
       }                                                                                            
    }    
                                                                                          

    因为没有类别代码了,现在每个类(Shape,Line,Rectangle,Circle)里面的所有属性就不会有时用得到有时用不到了。现在我们也可以给它们取一些好听点的名字了(比如在Line里面,p1这个属性可以改名为startPoint了)。现在四种异味只剩一种了,那就是,在drawShapes里面还是有一大串if-then-else-if。我们下一步,就是要去掉这长长的一串。
    

    消除代码异味:如何去掉一大串if-then-else-if(或者switch)       

    经常地,为了去掉if-then-else-if或者switch,我们需要先保证在每个条件分支下的要写的代码是一样的。在drawShapes这个方法里面,我们先以一个较抽象的方法(伪码)来写吧:
    
    class CADApp {                                                                                  
       void drawShapes(Graphics graphics, Shape shapes[]) {                                        
           for (int i = 0; i < shapes.length; i++) {                                                
               if (shapes[i] instanceof Line) {                                                    
                  画线条;                                                                    
               } else if (shapes[i] instanceof Rectangle) {                                        
                  画长方形;                                                              
               } else if (shapes[i] instanceof Circle) {                                            
                  画圆;                                                                  
               }                                                                                    
           }                                                                                        
       }                                                                                            
    }    
                                                                                          

    条件下的代码还是不怎么一样,不如再抽象一点:
    class CADApp {                                                                                  
       void drawShapes(Graphics graphics, Shape shapes[]) {                                        
           for (int i = 0; i < shapes.length; i++) {                                                
               if (shapes[i] instanceof Line) {                                                    
                  画出形状;                                                                  
               } else if (shapes[i] instanceof Rectangle) {                                        
                  画出形状;                                                                  
               } else if (shapes[i] instanceof Circle) {                                            
                  画出形状;                                                                  
               }                                                                                    
           }                                                                                        
       }                                                                                            
    }    
                                                                                          

    好,现在三个分支下的代码都一样了。我们也就不需要条件分支了:
    
    class CADApp {                                                                                  
       void drawShapes(Graphics graphics, Shape shapes[]) {
           for (int i = 0; i < shapes.length; i++) {
               画出形状;
           }    
       }        
    }      
    

    最后,将“画出形状”这个伪码写成代码吧:
    class CADApp {
       void drawShapes(Graphics graphics, Shape shapes[]) {
           for (int i = 0; i < shapes.length; i++) {
               shapes[i].draw(graphics);
           }    
       }        
    }        
  
    
    当然,我们需要在每种Shape的类里面提供draw这个方法:

    abstract class Shape {
       abstract void draw(Graphics graphics);
    }          
    class Line extends Shape {
       Point startPoint;
       Point endPoint;
       void draw(Graphics graphics) {
           graphics.drawLine(getStartPoint(), getEndPoint());
       }        
    }          
    class Rectangle extends Shape {
       Point lowerLeftCorner;
       Point upperRightCorner;
       void draw(Graphics graphics) {
           graphics.drawLine(...);
           graphics.drawLine(...);
           graphics.drawLine(...);
           graphics.drawLine(...);
       }        
    }          
    class Circle extends Shape {
       Point center;
       int radius;
       void draw(Graphics graphics) {
           graphics.drawCircle(getCenter(), getRadius());
       }        
    }
          

    将抽象类变成接口

    现在,看一下Shape这个类,它本身没有实际的方法。所以,它更应该是一个接口:


    interface Shape {                                                                               
       void draw(Graphics graphics);                                                                
    }                                                                                              
    class Line implements Shape {                                                                  
       ...                                                                                          
    }                                                                                              
    class Rectangle implements Shape {                                                              
       ...                                                                                          
    }                                                                                              
    class Circle implements Shape {                                                                
       ...                                                                                          
    }    
                                                                                          

    改进后的代码

    改进后的代码就像下面这样:
    
    interface Shape {                                                                               
       void draw(Graphics graphics);                                                                
    }                                                                                              
    class Line implements Shape {                                                                  
       Point startPoint;                                                                            
       Point endPoint;                                                                              
       void draw(Graphics graphics) {                                                              
           graphics.drawLine(getStartPoint(), getEndPoint());                                      
       }                                                                                            
    }                                                                                              
    class Rectangle implements Shape {                                                              
       Point lowerLeftCorner;                                                                      
       Point upperRightCorner;                                                                      
       void draw(Graphics graphics) {                                                              
           graphics.drawLine(...);                                                                  
           graphics.drawLine(...);                                                                  
           graphics.drawLine(...);                                                                  
           graphics.drawLine(...);                                                                  
       }                                                                                            
    }                                                                                              
    class Circle implements Shape {                                                                
       Point center;                                                                                
       int radius;                                                                                  
       void draw(Graphics graphics) {                                                              
           graphics.drawCircle(getCenter(), getRadius());                                          
       }                                                                                            
    }                                                                                              
    class CADApp {                                                                                  
       void drawShapes(Graphics graphics, Shape shapes[]) {                                        
           for (int i = 0; i < shapes.length; i++) {                                                
               shapes[i].draw(graphics);                                                            
           }                                                                                        
       }                                                                                            
    }
                                                                                              

    如果我们想要支持更多的图形(比如:三角形),上面没有一个类需要修改。我们只需要创建一个新的类Triangle就行了。



   另一个例子

    让我们来看一下另外一个例子。在当前的系统中,有三种用户:常规用户,管理员和游客。
    常规用户必须每隔90天修改一次密码(更频繁也行)。管理员必须每30天修改一次密码。游客就不需要修改了。
    常规用户跟管理员可以打印报表。
    先看一下当前的代码:

    class UserAccount {
       final static int USERTYPE_NORMAL = 0;
       final static int USERTYPE_ADMIN = 1;
       final static int USERTYPE_GUEST = 2;
       int userType;
       String id;  
       String name;
       String password;
       Date dateOfLastPasswdChange;
       public boolean checkPassword(String password) {
           ...    
       }          
    }              
    class InventoryApp {
       void login(UserAccount userLoggingIn, String password) {
           if (userLoggingIn.checkPassword(password)) {
               GregorianCalendar today = new GregorianCalendar();
               GregorianCalendar expiryDate = getAccountExpiryDate(userLoggingIn);
               if (today.after(expiryDate)) {
                  //提示用户修改密码
                  ...
               }  
           }      
       }          
       GregorianCalendar getAccountExpiryDate(UserAccount account) {
           int passwordMaxAgeInDays = getPasswordMaxAgeInDays(account);
           GregorianCalendar expiryDate = new GregorianCalendar();
           expiryDate.setTime(account.dateOfLastPasswdChange);
           expiryDate.add(Calendar.DAY_OF_MONTH, passwordMaxAgeInDays);
           return expiryDate;
       }          
       int getPasswordMaxAgeInDays(UserAccount account) {
           switch (account.getType()) {
               case UserAccount.USERTYPE_NORMAL:
                  return 90;
               case UserAccount.USERTYPE_ADMIN:
                  return 30;
               case UserAccount.USERTYPE_GUEST:
                  return Integer.MAX_VALUE;
           }      
       }          
       void printReport(UserAccount currentUser) {
           boolean canPrint;
           switch (currentUser.getType()) {
               case UserAccount.USERTYPE_NORMAL:
                  canPrint = true;
                  break;
               case UserAccount.USERTYPE_ADMIN:
                  canPrint = true;
                  break;                                                                            
               case UserAccount.USERTYPE_GUEST:                                                    
                  canPrint = false;                                                                
           }                                                                                        
           if (!canPrint) {                                                                        
               throw new SecurityException("You have no right");                                    
           }                                                                                        
           //打印报表
       }                                                                                            
    }
                                                                                              

     用一个对象代替一种类别(注意,之前是一个类代替一种类别)

    根据之前讲的解决方法,要去掉类别代码,我们只需要为每种类别创建一个子类,比如:
    
    abstract class UserAccount {                                                                    
       String id;                                                                                  
       String name;                                                                                
       String password;                                                                            
       Date dateOfLastPasswdChange;                                                                
       abstract int getPasswordMaxAgeInDays();                                                      
       abstract boolean canPrintReport();                                                          
    }                                                                                              
    class NormalUserAccount extends UserAccount {                                                  
       int getPasswordMaxAgeInDays() {                                                              
           return 90;                                                                              
       }                                                                                            
       boolean canPrintReport() {                                                                  
           return true;                                                                            
       }                                                                                            
    }                                                                                              
    class AdminUserAccount extends UserAccount {                                                    
       int getPasswordMaxAgeInDays() {                                                              
           return 30;                                                                              
       }                                                                                            
       boolean canPrintReport() {                                                                  
           return true;                                                                            
       }                                                                                            
    }                                                                                              
    class GuestUserAccount extends UserAccount {                                                    
       int getPasswordMaxAgeInDays() {                                                              
           return Integer.MAX_VALUE;                                                                
       }                                                                                            
       boolean canPrintReport() {                                                                  
           return false;                                                                            
       }                                                                                            
    }
                                                                                              

    但问题是,三种子类的行为(里面的代码)都差不多一样,getPasswordMaxAgeInDays这个方法就一个数值不同(30,90或者Integer.MAX_VALUE)。canPrintReport这个方法也不同在一个数值(true或false)。这三种用户类型只需要用三个对象代替就行了,无须特地新建三个子类了:

    class UserAccount {
       UserType userType;
       String id;  
       String name;
       String password;
       Date dateOfLastPasswdChange;
       UserType getType() {
           return userType;
       }          
    }              
    class UserType {
       int passwordMaxAgeInDays;
       boolean allowedToPrintReport;
       UserType(int passwordMaxAgeInDays, boolean allowedToPrintReport) {
           this.passwordMaxAgeInDays = passwordMaxAgeInDays;
           this.allowedToPrintReport = allowedToPrintReport;
       }          
       int getPasswordMaxAgeInDays() {
           return passwordMaxAgeInDays;
       }          
       boolean canPrintReport() {
           return allowedToPrintReport;
       }          
       static UserType normalUserType = new UserType(90, true);
       static UserType adminUserType = new UserType(30, true);
       static UserType guestUserType = new UserType(Integer.MAX_VALUE, false);
    }              
    class InventoryApp {
       void login(UserAccount userLoggingIn, String password) {
           if (userLoggingIn.checkPassword(password)) {
               GregorianCalendar today = new GregorianCalendar();
               GregorianCalendar expiryDate = getAccountExpiryDate(userLoggingIn);
               if (today.after(expiryDate)) {
                  //提示用户修改密码
                  ...
               }  
           }      
       }          
       GregorianCalendar getAccountExpiryDate(UserAccount account) {
           int passwordMaxAgeInDays = getPasswordMaxAgeInDays(account);
           GregorianCalendar expiryDate = new GregorianCalendar();
           expiryDate.setTime(account.dateOfLastPasswdChange);
           expiryDate.add(Calendar.DAY_OF_MONTH, passwordMaxAgeInDays);
           return expiryDate;
       }          
       int getPasswordMaxAgeInDays(UserAccount account) {
           return account.getType().getPasswordMaxAgeInDays();
       }          
       void printReport(UserAccount currentUser) {
           boolean canPrint;
           canPrint = currentUser.getType().canPrintReport();
           if (!canPrint) {
               throw new SecurityException("You have no right");
           }      
           //打印报表.
       }          
    }
                                                                                              

    注意到了吧,用一个对象代替类别,同样可以移除switch或者if-then-else-if。      

总结一下类别代码的移除

    要移动一些类别代码和switch表达式,有两种方法:                            
    1.用基于同一父类的不同子类来代替不同的类别。                                        
    2.用一个类的不同对象来代替不同的类别。
    当不同的类别具有比较多不同的行为时,用第一种方法。当这些类别的行为非常相似,或者只是差别在一些值上面的时候,用第二个方法。

     普遍的代码异味
    类别代码和switch表达式是比较普遍的代码异味。此外,还有其他的代码异味也很普遍。
    下面是大概的异味列表:
    代码重复
    太多的注释
    类别代码(type code)

   switch或者一大串if-then-else-if
    想给一个变量,方法或者类名取个好名字时,也怎么也取不好    
    用类似XXXUtil, XXXManager, XXXController 和其他的一些命名
    在变量,方法或类名中使用这些单词“And”,“Or”等等  
    一些实例中的变量有时有用,有时没用
    一个方法的代码太多,或者说方法太长
    一个类的代码太多,或者说类太长
    一个方法有太多参数
    两个类都引用了彼此(依赖于彼此)

资源
下载PDF文件
0 0
原创粉丝点击