Franzi Sauerwein @Singsalad

Refactoring mount doom

A team's dilemma

@singsalad

Uncertainty is fear 😧

@singsalad

Refactoring legacy code

  • How did we get here?
  • How do we deal with it?
  • How do we improve?

@singsalad

Refactoring legacy code

  • How did we get here?
  • How do we deal with it?
  • How do we improve?

@singsalad

It's ....'s fault

@singsalad

Prime directive

 

Regardless of what we discover, we understand and truly believe that everyone did the best job they could, given what they knew at the time, their skills and abilities, the resources available, and the situation at hand.

--Norm Kerth, Project Retrospectives: A Handbook for Team Review

@singsalad

Reasons for Technical Debt

  • delivery pressure
  • lack of skills
  • unclear requirements

@singsalad

@singsalad

Where do we start?

  • Critical business cases
  • The code where your next feature goes
  • Split into validated peaceful zone and war zone outside (@ziobrando)
  • Mikado method

Be clear about the WHY

@singsalad

What do we need

  • Map (documentation 🤦)
  • Safety (tests, good work environment)
  • Tools & Supplies (IDE, skills)
  • Guide (Product person 🔁)

@singsalad

@singsalad

Refactoring legacy code

  • How did we get here?
  • How do we deal with it?
  • How do we improve?

@singsalad

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();
        }

    }

@singsalad

@singsalad

@singsalad

Different scenarios

Guided vs. Unguided

@singsalad

Different scenarios

Guided vs. Unguided

  • Manual Exploring
  • Golden Master
  • Higher Level Tests

@singsalad

Manual exploratory testing

@singsalad

Safety with Golden Master

Goal: fast feedback & basic safety net

  1. redirect output
  2. run the original system with various inputs
  3. run modified code with the same inputs
  4. compare the original output with refactoring

@singsalad

@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

@singsalad

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

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

Golden master output

@singsalad

Modularity

Class

under test

Dependency injection

@singsalad

Modularity

Class under test

Original

collaborator

Dependency injection

Class

under test

@singsalad

Modularity

Original

collaborator

Mocked

Collaborator

Dependency injection

Class

under test

@singsalad

Encapsulate external dependencies

public class ConsolePrinter implements Printer {
    @Override
    public void print(Object objectToPrint) {
        System.out.println(objectToPrint);
    }
}

@singsalad

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);
}

Encapsulate external dependencies

@singsalad

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);
}

Exploratory testing

@singsalad

@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"
   ));
}

Exploratory testing

@singsalad

Exploratory testing

@singsalad

Different scenarios

Guided vs. Unguided

  • Manual Exploring
  • Golden Master
  • Higher Level Tests

@singsalad

Different scenarios

Guided vs. Unguided

  • Reveal intent
  • Fix abstractions
  • Improve design

@singsalad

Reveal intent - Naming

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];

@singsalad

Fix abstractions

do:

  1. take flour, water, ...
  2. mix
  3. take tomatos, spices,...
  4. mix
  5. take vegetables, cheese,...
  6. chop
  7. put dough on baking tray
  8. put in oven
  9. ....

make pizza:

  1. prep dough
  2. prep sauce
  3. prep toppings
  4. assemble
  5. bake
  6. eat

prep dough:

  1. take flour, water, ...
  2. mix

....

@singsalad

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();
        }

    }

@singsalad

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();
   }
}

@singsalad

What's next?

Roll

Penalty Box

Player

Category

Move

Board

Question

Position

Gold Coins

@singsalad

Different scenarios

Guided vs. Unguided

  • Reveal intent
  • Fix abstractions
  • Improve design

@singsalad

Refactoring legacy code

  • How did we get here?
  • How do we deal with it?
  • How do we improve?

@singsalad

How much effort did we put in?

@singsalad

Uncertainty is fear 😧

@singsalad

Learn

Teach

Invest

@singsalad

Explore & understand

Document through code

Improve continuously

@singsalad

Thank you

github.com/franziskas/trivia/tree/conditionals

slides.com/franziskasauerwein/refactoring-mount-doom

refactoring.com

@singsalad

kata-log.rocks/refactoring

Made with Slides.com