MarRover代码重构

上周末参与公司的招聘面试，跟其中的一个候选人pair编程，但由于面试时间有限，实现了新需求后，只重构了代码的一小部分，所以在面试之后，我就继续将剩余的部分重构完成。重构的整个过程可以clone一份看看（[e-mail]:kent-wu/MarRoversRefactor.git）

题目大概的意思是这样的，有一个机器人在一个平原里，我们用坐标轴给机器人定位。这个平原的大小是有限的，(5,5)代表x的上限为5，y的上限为5。机器人有初始位置和朝向，例如(1,2,N)，在坐标轴的点(1,2)上，面朝北。我们可以给它下指令(M: 前进，B: 后退，L: 左转，R: 右转)，执行一系列指令之后，得出机器人的位置和朝向。例如初始状态为(1,2,N)，执行指令LMLMLMLMM后状态变为(1,3,N)。

原始代码如下

public class Client {//输入指令时请输入大写的字母，小写public static void main(String[] args) throws Exception {int initx=0;int inity=0;int boundaryX=0;int boundaryY=0;Robot r=new Robot();BufferedReader br = new BufferedReader(new InputStreamReader(System.in));         String strBoundary = null;         System.out.println("Enter your Boundary x and y:");         strBoundary = br.readLine();        String[] boundaryArr=strBoundary.split(" ");        if(!idDigit(boundaryArr[0]) || !idDigit(boundaryArr[1])){        System.out.println("Boundary x and y are must be digit,and input them again:");                 //重新输入的代码        }        //初始化边界点        Robot.X=Integer.parseInt(boundaryArr[0]);        Robot.Y=Integer.parseInt(boundaryArr[1]);        String strInitPos = null;         System.out.println("Enter your robot initial x,y and direstion:");         strInitPos = br.readLine();        String[] initPos=strInitPos.split(" ");        //加入检查输入合法性的代码                //初始化当前Robot的位置        r.x=Integer.parseInt(initPos[0]);        r.y=Integer.parseInt(initPos[1]);        r.dir=Character.toUpperCase(initPos[2].charAt(0));        //获得控制动作        String strOrder = null;         while(true){                    System.out.println("Enter your Robot orders:");             strOrder = br.readLine();            //加入控制命令合法性检查            String result=r.control(strOrder);            System.out.println(result);            if("RIP".equals(result)){            System.exit(0);            }        }        }public static boolean idDigit(String str){Pattern pattern = Pattern.compile("[0-9]*");    return pattern.matcher(str).matches();}}

public class Robot {public static int X;public static int Y;//positionpublic int x;public int y;//directionpublic char dir;public String control(String order){char[] orders=order.toCharArray();for(char tmp : orders){tmp=Character.toUpperCase(tmp);if(tmp == 'L' || tmp == 'R'){this.setDir(tmp);}else{this.move(tmp);if(this.checkError()){return "RIP";}}}//for endreturn this.x+" "+this.y+" "+this.dir;}private void setDir(char change){if(change=='L'){if(this.dir=='N'){this.dir='W';}else if(this.dir=='S'){this.dir='E';}else if(this.dir=='W'){this.dir='S';}else if(this.dir=='E'){this.dir='N';}}else{if(this.dir=='N'){this.dir='E';}else if(this.dir=='S'){this.dir='W';}else if(this.dir=='W'){this.dir='N';}else if(this.dir=='E'){this.dir='S';}}}private void move(char des){if(des == 'M'){if (this.dir == 'N') {this.y += 1;} else if (this.dir == 'S') {this.y -= 1;} else if (this.dir == 'W') {this.x -= 1;} else if (this.dir == 'E') {this.x += 1;}}else{if (this.dir == 'N') {this.y -= 1;} else if (this.dir == 'S') {this.y += 1;} else if (this.dir == 'W') {this.x += 1;} else if (this.dir == 'E') {this.x -= 1;}}}private boolean checkError(){if(this.x>X || this.x<0 || this.y>Y || this.y<0){return true;}return false;}}

在重构之前，一定要有测试，不然重构的过程中可能会带入bug。这里就不展示测试代码了，有兴趣的可以到github去clone看看，注释其实不是特别提倡，因为注释的维护成本比较大，很容易导致方法的实际意图与注释不一致的情况。所以我个人认为，与其写注释，不如让方法和变量的命名更加清晰，这样更好。在《代码整洁之道》一书中有对注释的好坏进行详细的阐述，有兴趣可以去看看。
而这份代码的注释其实也没有什么很大的作用，所以我先把注释去掉。

这份代码主要有两个类，Client类和Robot类。我们先从Client类开始重构。
首先，Client类里面又四个无用的变量，initx，inity，boundaryX，boundaryY，所以先删掉。

这个main方法很长很复杂，所以第一个重构对象就是main方法。
第一步，将main中的代码按照功能，抽取成三个方法，init，initRobot和outputResult。
第二步，再对抽取出来的三个方法进一步重构。
重构后的效果如下：

public class Client {public static void main(String[] args) throws Exception {Robot robot=new Robot();BufferedReader br = new BufferedReader(new InputStreamReader(System.in));        init(br);        initRobot(robot, br);        while(true){            outputResult(robot, br);        }        }    private static void outputResult(Robot robot, BufferedReader br) throws IOException {        System.out.println("Enter your Robot orders:");        String result=robot.control(br.readLine());        System.out.println(result);    }    private static void initRobot(Robot robot, BufferedReader br) throws IOException {        System.out.println("Enter your robot initial x,y and direstion:");        String[] initPos= br.readLine().split(" ");        robot.x=Integer.parseInt(initPos[0]);        robot.y=Integer.parseInt(initPos[1]);        robot.dir=Character.toUpperCase(initPos[2].charAt(0));    }    private static void init(BufferedReader br) throws IOException {        System.out.println("Enter your Boundary x and y:");        String[] boundaryArr= br.readLine().split(" ");        if(!idDigit(boundaryArr[0]) || !idDigit(boundaryArr[1])){        System.out.println("Boundary x and y are must be digit,and input them again:");        }        Robot.X=Integer.parseInt(boundaryArr[0]);        Robot.Y=Integer.parseInt(boundaryArr[1]);    }    public static boolean idDigit(String str){Pattern pattern = Pattern.compile("[0-9]*");    return pattern.matcher(str).matches();}}

运行测试，全部通过。
接下来，就是重构Robot类。
首先，我们看到就是变量的命名不是很好，我们稍微修改一下。
接下来，我们看到，整一个Robot类，几乎都是if-else，所以接下来就是重构if-else。
第一步，我们将setCurrentDirection和move方法里面的if-else抽取出四个方法，让代码意图更清晰。

public class Robot {    public static int x_bound;    public static int y_bound;    public int x;    public int y;    public char currentDirection;    public String control(String order) {        char[] orders = order.toCharArray();        for (char tmp : orders) {            tmp = Character.toUpperCase(tmp);            if (tmp == 'L' || tmp == 'R') {                this.setCurrentDirection(tmp);            } else {                this.move(tmp);                if (this.checkError()) {                    return "RIP";                }            }        }        return this.x + " " + this.y + " " + this.currentDirection;    }    private void setCurrentDirection(char change) {        if (change == 'L') {            turnLeft();        } else {            turnRight();        }    }    private void move(char des) {        if (des == 'M') {            moveForward();        } else {            moveBack();        }    }    private void turnRight() {        if (this.currentDirection == 'N') {            this.currentDirection = 'E';        } else if (this.currentDirection == 'S') {            this.currentDirection = 'W';        } else if (this.currentDirection == 'W') {            this.currentDirection = 'N';        } else if (this.currentDirection == 'E') {            this.currentDirection = 'S';        }    }    private void turnLeft() {        if (this.currentDirection == 'N') {            this.currentDirection = 'W';        } else if (this.currentDirection == 'S') {            this.currentDirection = 'E';        } else if (this.currentDirection == 'W') {            this.currentDirection = 'S';        } else if (this.currentDirection == 'E') {            this.currentDirection = 'N';        }    }    private void moveBack() {        if (this.currentDirection == 'N') {            this.y -= 1;        } else if (this.currentDirection == 'S') {            this.y += 1;        } else if (this.currentDirection == 'W') {            this.x += 1;        } else if (this.currentDirection == 'E') {            this.x -= 1;        }    }    private void moveForward() {        if (this.currentDirection == 'N') {            this.y += 1;        } else if (this.currentDirection == 'S') {            this.y -= 1;        } else if (this.currentDirection == 'W') {            this.x -= 1;        } else if (this.currentDirection == 'E') {            this.x += 1;        }    }    private boolean checkError() {        if (this.x > x_bound || this.x < 0 || this.y > y_bound || this.y < 0) {            return true;        }        return false;    }}

第二步，我们看到turnLeft，turnRight，moveForward和moveBack都是高度相似的，这四个方法重复率很高。
这四个方法之所以重复，就是因为需要判断’N’,’S’,’E’,’W’，所以我们就需要将用字符转换成用数字代表方位。
建立char direction = new char[]{'N','E','S','W’}，按照顺时针排放方位；
建立一个Map directionMap = new HashMap()来将用字符转换为用数字来代表方位。
这样我们就可以0代表N，1代表E，2代表，3代表W，所以之前的currentDirection的类型和setCurrentDirection方法也需要有些改变。
重构的原则是可控的地小步重构，直接修改currentDirection的类型会导致一大片代码需要修改，所以我们先引入一个变量currentDirectionInt。
旧的代码可以被新的代码替换掉，而且新的代码都通过测试，我们再把旧代码删除，然后再将currentDirectionInt重命名为currentDirection即可。
引入新变量currentDirectionInt和setCurrentDirectionInt方法，要跑测试保证没有影响原有功能。

第三步，之前turnLeft需要先判断是现在是什么朝向，然后再按照朝向判断左转后是什么方向。而我们现在用数字表示朝向，所以左转其实就是相当于-1。
不过当朝向为N的时候，0减1就成了-1了，为了避免这种情况，左转应该是(currentDirectionInt+3)%4
所以turnleft可以简化成一行代码currentDirectionInt = (currentDirectionInt + 3) % 4;
类似地turnRight就可以简化成currentDirectionInt = (currentDirectionInt + 1) % 4;
同样地，我们先不在原有的方法修改，我们先新加方法turnLeftInt和turnRightInt

第四步，重构moveForward和moveBack。
还有一样的原因，需要判断朝向才能知道x轴和y轴是加1还是减1。
其实如果朝向为N或者S的时候，前进或者后退，x轴的数值都是不会变的，变得只是y轴的值，而每次都是加或者减1。
因为我们以数字表示方位，所以当处于不同朝前进，y轴的变化可以这样表示y_moves = {1,0,-1,0}，同理，x轴的变化可以表示为x_moves = {0,1,0,-1}。
因此，moveForward可以简化为       
        x += x_moves[currentDirection];
        y += y_moves[currentDirection];
moveBack简化为
        x -= x_moves[currentDirection];
        y -= y_moves[currentDirection];

同样，两个新方法moveForwardInt和moveBackInt。

第五步，重构control方法。
还是老思路，先复制一个control方法，名为controlRefactor。
将原来调用turnLeft、turnRight、moveForward和moveBack的，改为调用turnLeftInt、turnRightInt、moveForwardInt和moveBackInt。

第六步，接下来，就是将原来Client和RobotTest里面调用Robot.control的改为调用Robot.controlRefactor
然后跑一下测试，验证是否重构是否正确。

第七步，测试通过了，说明我们的重构完成，现在就需要把旧的方法变量删除掉。
删除之后，再跑测试保证没用删错。

第八步，将重构时候的新方法和变量重命名，之后再跑测试。

至此，Robot类的重构也基本完成了

public class Robot {    public static int x_bound;    public static int y_bound;    public int x;    public int y;    private int currentDirection;    private Map directionMap;    private char[] direction = new char[]{'N', 'E', 'S', 'W'};    private int[] x_moves = new int[]{0, 1, 0, -1};    private int[] y_moves = new int[]{1, 0, -1, 0};    public Robot() {        directionMap = new HashMap();        directionMap.put('N',0);        directionMap.put('E',1);        directionMap.put('S',2);        directionMap.put('W',3);    }    public String control(String orderString) {        char[] orders = orderString.toCharArray();        for (char order : orders) {            order = Character.toUpperCase(order);            switch (order) {                case 'L':                    turnLeft();                    break;                case 'R':                    turnRight();                    break;                case 'M':                    moveForward();                    break;                case 'B':                    moveBack();                    break;            }            if (this.checkError()) {                return "RIP";            }        }        return x + " " + y + " " + direction[currentDirection];    }    public void setCurrentDirection(char currentDirection) {        this.currentDirection = (Integer) directionMap.get(currentDirection);    }    public void turnLeft() {        currentDirection = (currentDirection + 3) % 4;    }    public void turnRight() {        currentDirection = (currentDirection + 1) % 4;    }    public void moveForward() {        x += x_moves[currentDirection];        y += y_moves[currentDirection];    }    public void moveBack() {        x -= x_moves[currentDirection];        y -= y_moves[currentDirection];    }    private boolean checkError() {        if (this.x > x_bound || this.x < 0 || this.y > y_bound || this.y < 0) {            return true;        }        return false;    }}

重构后的Robot类还是不是很完美，因为有个非常不好看的switch-case。可以使用多态来消失这个switch-case。
我在这里就不再展示了怎么重构了（因为这篇博客已经够长了，再长一点的话就没有人看了），有兴趣可以阅读《重构：改善既有代码的设计》这书中关于使用多态将switch-case去除的例子。
如果Robot中得switch-case用多态去掉后，对设计模式比较熟悉的同学可能看出来，代码长得有点命令模式的味道。所以，这份代码可以更进一步重构成使用命令模式，重构所使用的步骤也不是很复杂，有待同学们去亲手实践一下。

详细的重构过程，可以在github上面clone一份，然后看每一个commit。（[e-mail]:kent-wu/MarRoversRefactor.git)

最后，总结一下重构的过程：

1、如果没有测试，补充测试

2、去掉一下无用的方法或者变量

3、将比较大的方法通过抽取方法的重构手法，让方法的意图清晰

4、小步快跑地重构原来的方法并在调用的地方做相应的替换