上周末参与公司的招聘面试,跟其中的一个候选人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、小步快跑地重构原来的方法并在调用的地方做相应的替换