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
- redirect output
- run the original system with various inputs
- run modified code with the same inputs
- 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:
- take flour, water, ...
- mix
- take tomatos, spices,...
- mix
- take vegetables, cheese,...
- chop
- put dough on baking tray
- put in oven
- ....
make pizza:
- prep dough
- prep sauce
- prep toppings
- assemble
- bake
- eat
prep dough:
- take flour, water, ...
- 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
Refactoring Mount Doom - Tackling Legacy Code
By Franziska Sauerwein
Refactoring Mount Doom - Tackling Legacy Code
We’ve all had that nightmare where you are try to get to your destination, and keeping moving, trying different things, but for various reasons, you never arrive. Some refactorings are like that - you extract methods, name constants, increase readability… In short, you spend a lot of time cleaning up - but you never get to a good place with the code. In this talk, I will show you how to refactor for the right reasons and the right methods to use your time efficiently. Let’s make working with that code easier in the future.
