I had a problem in refactoring this code. how do I extract this long method? I don't know where the smell code in these codes, can you guys help me to figure it out these smell code and how to refactor this code?
private boolean placingFlag = false;
protected Scanner scan = new Scanner(System.in);
public void play() {
clear();
print();
if(placingFlag) {
System.out.println("currently your command is for placing/unplacing flag. Type 'switch' to start opening squares");
} else {
System.out.println("currently your command is for opening square. Type 'switch' to placing flag");
}
do {
System.out.print("Input coordinate to command: ");
String input = scan.nextLine().trim();
if(input.equalsIgnoreCase("switch")) {
placingFlag = !placingFlag;
break;
}
if(input.length() != 2) {
System.out.println("invalid coordinate");
continue;
}
char c1 = input.charAt(0);
char c2 = input.charAt(1);
int x = c1 - 'A';
int y = c2 - '1';
if(x < 0 || x >= gameSetting.getWidth()) {
System.out.println("invalid coordinate");
continue;
}
if(y < 0 || y >= gameSetting.getHeight()) {
System.out.println("invalid coordinate");
continue;
}
if(!placingFlag) {
if(board[y][x].flagged){
System.out.println("cannot open flagged square");
continue;
}
if(board[y][x].getType().equalsIgnoreCase("mine")) {
this.lose();
return;
}
open(x, y);
if(isWin()) {
this.win();
return;
}
} else {
if(board[y][x].getType().equalsIgnoreCase("number")) {
System.out.println("opened squared cannot be flagged");
continue;
}
board[y][x].flagged = !board[y][x].flagged;
}
break;
}while(true);
play();
}
please guys give me enlightment for these code how to extract these long method :)
It's not too bad. One thing you could do - separate reading input from playing based on that input:
private static class GameMove {
int x;
int y;
boolean switchFlagPlacingMode;
}
private GameMove readNextMove() {
// read in using the scanner
// and validate the coordinates
}
Another thing - while(true)
is definitely a code smell, as is the number of continue
, break
, and return
statements you have - they make the code hard to trace. Consider something like
boolean gameOver = false;
while(!gameOver) {
boolean isMoveValid = false;
GameMove nextMove = readNextMove();
while(!isMoveValid) {
// validate, set isMoveValid
// if invalid: nextMove = readNextMove();
}
// process the move
if (isWin()) {
this.win();
gameOver = true;
}
// similar for loss
}