@talboomerik

@singsalad

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

Made with Slides.com