上周末参与公司的招聘面试,跟其中的一个候选人pair编程,但由于面试时间有限,实现了新需求后,只重构了代码的一小部分,所以在面试之后,我就继续将剩余的部分重构完成。重构的整个过程可以clone一份看看(git@github.com:wjh-thoughtworks/MarRoversRefactor.git)
题目大概的意思是这样的,有一个机器人在一个平原里,我们用坐标轴给机器人定位。这个平原的大小是有限的,(5,5)代表x的上限为5,y的上限为5。机器人有初始位置和朝向,例如(1,2,N),在坐标轴的点(1,2)上,面朝北。我们可以给它下指令(M: 前进,B: 后退,L: 左转,R: 右转),执行一系列指令之后,得出机器人的位置和朝向。例如初始状态为(1,2,N),执行指令LMLMLMLMM后状态变为(1,3,N)。
原始代码如下
?
class="java">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; //position public int x; public int y; //direction public 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 end return 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。(git@github.com:wjh-thoughtworks/MarRoversRefactor.git)
最后,总结一下重构的过程:
1、如果没有测试,补充测试
2、去掉一下无用的方法或者变量
3、将比较大的方法通过抽取方法的重构手法,让方法的意图清晰
4、小步快跑地重构原来的方法并在调用的地方做相应的替换
?
?
?
?
?
?