refactoring--除去代码异味(bad smell)(1)

来源:互联网 发布:醍醐灌顶的一句话 知乎 编辑:程序博客网 时间:2024/03/29 22:57

    异味这个词, 可能有点抽象, 我们先看一下下面的例子
这是一个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 就行了 。