MarRover代码重构
上周末参与公司的招聘面试，跟其中的一个候选人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)。
原始代码如下
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、小步快跑地重构原来的方法并在调用的地方做相应的替换
上一篇： HTML5/CSS3翻转动画（二）
下一篇： 用dTree组件生成无限级导航树