Refactoring mount doom
Mount Doom
public void roll(int roll) {
System.out.println(players.get(currentPlayer) + " is the current player");
System.out.println("They have rolled a " + roll);
if (inPenaltyBox[currentPlayer]) {
if (roll % 2 != 0) {
isGettingOutOfPenaltyBox = true;
System.out.println(players.get(currentPlayer) + " is getting out of the penalty box");
places[currentPlayer] = places[currentPlayer] + roll;
if (places[currentPlayer] > 11) places[currentPlayer] = places[currentPlayer] - 12;
System.out.println(players.get(currentPlayer)
+ "'s new location is "
+ places[currentPlayer]);
System.out.println("The category is " + currentCategory());
askQuestion();
} else {
System.out.println(players.get(currentPlayer) + " is not getting out of the penalty box");
isGettingOutOfPenaltyBox = false;
}
} else {
places[currentPlayer] = places[currentPlayer] + roll;
if (places[currentPlayer] > 11) places[currentPlayer] = places[currentPlayer] - 12;
System.out.println(players.get(currentPlayer)
+ "'s new location is "
+ places[currentPlayer]);
System.out.println("The category is " + currentCategory());
askQuestion();
}
}
Reveal intent
int[] places = new int[6];
int[] purses = new int[6];
boolean[] inPenaltyBox = new boolean[6]; int[] places = new int[MAXIMUM_AMOUNT_OF_PLAYERS];
int[] numberOfGoldCoins = new int[MAXIMUM_AMOUNT_OF_PLAYERS];
boolean[] inPenaltyBox = new boolean[MAXIMUM_AMOUNT_OF_PLAYERS];
Manual exploratory testing
Golden master
@Test
public void run_golden_master() throws IOException {
writeOutputTo(TESTRUN_FILE_NAME);
runGameTimes(500);
assertThat(readFile(TESTRUN_FILE_NAME),
is(readFile(MASTER_FILE_NAME)));
}Golden master
private void writeOutputTo(String filename)
throws FileNotFoundException {
System.setOut(new PrintStream(
new FileOutputStream(filename)));
}
private String readFile(String filename)
throws IOException {
return new String(readAllBytes(
Paths.get(filename)));
}Golden master
private void runGameTimes(int numberOfGamesRun) {
for (int seed = 0; seed < numberOfGamesRun; seed++) {
Random rand = new Random(seed);
runOneGame(rand);
}
}
private void runOneGame(Random rand) {
Game aGame = createGame();
boolean notAWinner;
do {
aGame.executeMove(rand.nextInt(5) + 1);
notAWinner = checkIfWinner(answerWasWrong(rand), aGame);
} while (notAWinner);
}Golden master
Encapsulate external dependencies
public class ConsolePrinter implements Printer {
@Override
public void print(Object objectToPrint) {
System.out.println(objectToPrint);
}
}Encapsulate external dependencies
public Game(Printer printer) {
this.printer = printer;
...
}
public void executeMove(int roll) {
...
print("They have rolled a " + roll);
...
}
private void print(Object output) {
printer.print(output);
}System tests
private Game game;
private List<String> printedLines;
private Printer printer = new Printer() {
@Override
public void print(Object objectToPrint) {
printedLines.add(objectToPrint.toString());
}
};
@Before
public void initialise() {
printedLines = new ArrayList<>();
game = new Game(printer);
}System tests
@Test
@Parameters({"0,Pop", "1,Science", "2,Sports", "3,Rock"})
public void
prints_game_moves_and_asks_categorised_questions(
int rolled, String category) {
addPlayers(2);
game.executeMove(rolled);
assertThat(printedLines, contains(
"player1 was added",
"They are player number 1",
"player2 was added",
"They are player number 2",
"player1 is the current player",
"They have rolled a " + rolled,
"player1's new location is " + rolled,
"The category is " + category,
category + " Question 0"
));
}System tests
Remember?
public void roll(int roll) {
System.out.println(players.get(currentPlayer) + " is the current player");
System.out.println("They have rolled a " + roll);
if (inPenaltyBox[currentPlayer]) {
if (roll % 2 != 0) {
isGettingOutOfPenaltyBox = true;
System.out.println(players.get(currentPlayer) + " is getting out of the penalty box");
places[currentPlayer] = places[currentPlayer] + roll;
if (places[currentPlayer] > 11) places[currentPlayer] = places[currentPlayer] - 12;
System.out.println(players.get(currentPlayer)
+ "'s new location is "
+ places[currentPlayer]);
System.out.println("The category is " + currentCategory());
askQuestion();
} else {
System.out.println(players.get(currentPlayer) + " is not getting out of the penalty box");
isGettingOutOfPenaltyBox = false;
}
} else {
places[currentPlayer] = places[currentPlayer] + roll;
if (places[currentPlayer] > 11) places[currentPlayer] = places[currentPlayer] - 12;
System.out.println(players.get(currentPlayer)
+ "'s new location is "
+ places[currentPlayer]);
System.out.println("The category is " + currentCategory());
askQuestion();
}
}More readable?
public void executeMove(int roll) {
print(players.get(currentPlayer) + " is the current player");
print("They have rolled a " + roll);
if (inPenaltyBox[currentPlayer]) {
boolean rollIsOdd = roll % 2 != 0;
if (rollIsOdd) {
playerGetsOutOfPenaltyBox();
moveTheCurrentPlayer(roll);
askTheNextQuestion();
} else {
playerDoesNotGetOutOfPenaltyBox();
}
} else {
moveTheCurrentPlayer(roll);
askTheNextQuestion();
}
}Refactored conditionals
- Extract the variable rollsOdd to explain the conditional expression
- Extract player gets out of penalty box behaviour into a method
- Extract player does not get out of penalty box behaviour into method
- Extract the end of the board conditional expression into a variable
Refactored conditionals
- Extract the behaviour of really executing a move into a method
- Extract move the current player behaviour into a method
- Extracted the ask the next question behaviour into a method
- Inlined the move the player and ask the next question
What's next?
Roll
Penalty Box
Player
Category
Move
Board
Questions
Position
Gold Coins
Thank you
github.com/franziskas/trivia/tree/conditionals
slides.com/franziskasauerwein/refactoring
Refactoring
By Franziska Sauerwein
