At the We Are Developers World Congress 2018, I gave a talk about how to deal with legacy code using the Mikado Method. So, here is the concrete example I gave in my talk.

The Mikado Method

The Mikado Method gives us a way to change legacy code in small, safe steps. Read the linked post for a detailed description. To summarize:

1. Write down your current goal
2. Try to reach that goal directly
3. If you fail
   1. Write down everything that prevents you from reaching your current goal as a sub-goal
   2. Revert your changes
   3. For each sub-goal, repeat 1
4. If you succeed
   1. Commit your changes
   2. Repeat with the next goal

This gives you a tree where all the leaves are immediately solvable, and when you solved them, you can try and solve the goals higher up in the tree. You might discover more problems that prevent you from reaching your goal - Then you just add another branch to the tree.

In the end, you will - hopefully - have solved your original goal. And you will have done so in small, safe-to-fail steps.

The Legacy Code

I used the Baby Steps Timer as the starting point for this exercise. It’s a pretty nasty piece of code where every part of the code is coupled to something else in the code. So, whenever you try to change something, something else breaks.

Before I started with the mikado method, I wrote some characterization tests (maybe I’ll write about it in a future blog post). They are still a little bit flaky - they sometimes fail without a reason - because of all the threading involved.

But it is absolutely crucial to have them: With the mikado method, we must have a way to quickly check whether our change broke something. And the compiler is not enough for that.

Move Timer Thread to Own File

This commit on the branch characterization-tests is where I started with the actual refactoring. I have at least some tests in place now. So, I created a new branch called mikado-method and I also wrote down my main goal:

…and tried to move the TimerThread out of the main class, using the refactoring tools. This resulted in code that does not compile:

Now, let me repeat that this - the code not compiling - is totally expected: With the mikado method, you just try to reach the goal in the most straight-forward way. You expect things to break. But you do this to identify your sub-goals. And then, you revert all your changes, back to the last working version.

Now I was supposed to write down everything that prevents me from achieving my goal. But in such a situation, I do not write down every compiler error. I write down some high-level design changes that will help me reach my goal. In this case, the compiler errrors belong to two categories: Stuff the timer needs for their own logic to work and stuff the timer needs for rendering the result.

Don’t forget to write those sub-goals down. It is not enough to just identify them and hope you will remember. I prefer paper for writing things like that down, but a simple text file or a mind mapping tool will do too.

Timer Logic

This was a quick one… I moved the three variables that are related to the business logic to the timer thread inner class:

private static final class TimerThread extends Thread {
    private static boolean timerRunning;
    private static long currentCycleStartTime;
    private static String lastRemainingTime;

And in the rest of the code, I access them through the TimerThread class:

} else if("command://stop".equals(e.getDescription())) {
    TimerThread.timerRunning = false;
    timerFrame.setAlwaysOnTop(false);

Rendering Logic

I moved all the code related to rendering to a TimerRenderer class.

public static class TimerRenderer {
    static JTextPane timerPane;
    private static String bodyBackgroundColor = BACKGROUND_COLOR_NEUTRAL;
    private static JFrame timerFrame;

    //...

    public static String getRemainingTimeCaption(final long elapsedTime) {
        //...
    }

    private static String createTimerHtml(final String timerText, final String bodyColor, final boolean running) {
        //...
    }
}

I also had to create an interface that the timer thread can use:

public static class TimerRenderer {
    //...
    public static String getBodyBackgroundColor() {
        return bodyBackgroundColor;
    }
    public static void update(String remainingTime, String bodyBackgroundColor, boolean timerRunning) {
        TimerRenderer.bodyBackgroundColor = bodyBackgroundColor;
        timerPane.setText(createTimerHtml(remainingTime, bodyBackgroundColor, true));
        timerFrame.repaint();
    }
    //...
}

The timer thread now uses only this interface when updating the timer:

while(timerRunning) {
    long elapsedTime = wallclock.currentTimeMillis() - currentCycleStartTime;

    //...
    String bodyBackgroundColor = timerRenderer.getBodyBackgroundColor();
    //...
    
    String remainingTime = timerRenderer.getRemainingTimeCaption(elapsedTime);
    if(!remainingTime.equals(lastRemainingTime)) {
        //...
    }
    timerRenderer.update(remainingTime, bodyBackgroundColor, true);
    //...
}

Note: This change introduces a subtle defect. Apparently, I forgot one characterization test that would have cought it. It is very hard to know which characterization tests to write, and whether you have enough. But you still should write them!

Can you spot the defect?

More Problems

Now my two original sub-goals were complete. So, I tried to move the TimerThread to its own class again. But I noticed two more problems:

I must pass the timer renderer to the timer thread as a constructor parameter - otherwise, the renderer cannot access it anymore, once it lives in a different file.

public TimerThread(BabystepsTimer.TimerRenderer timerRenderer) {
    this.timerRenderer = timerRenderer;
}

And I also created an interface to stop and to reset the timer, so that the main class does not have to access private variables of the timer thread anymore:

public static void stopTimer() {
    timerRunning = false;
}

public static void resetTimer(long newTime) {
    currentCycleStartTime = newTime;
}

I also had to change the access of one constant from private to public.

Done!

And now it was finally possible: I was able to move the TimerThread out of the main class, into its own file.

You can see the end result in the mikado-method branch of the babysteps-timer repository.

Maybe I could also have done this by doing a “refactor by compiler error”: Just do something, and then fix all the errors, do more, fix more compiler errors.

But this would definitely have been harder. The nice thing about the mikado method is that it is very safe. The code always compiles. You can always run the tests.

And when you do something that does not work right out of the box, you undo and solve the problems first. This allows you to work in small, safe steps, and to identify the steps as you go.

If you liked this post, you might be interested in my services: I help teams to get better at developing high-quality software. Just contact me!

Other blog posts you might be interested in:

• Slow Down to Move Faster
• Stuck in a Bad Job 2: Fuck-You Money
• Brain Based Training

Related posts in the category “Legacy Code”:

• Legacy Code: The Mikado Method