The Way of Refactoring

If I work purely for today, tomorrow I will be utterly unable to work.

– A Certain Master

Programmers must program for the future. Code refactoring is a topic programmers can never escape: the moment your software is written, refactoring becomes inevitable. Why do we labor so hard to keep abstracting when building a system, doing everything in our power to make our code reusable? Put plainly, we’re spending today’s time so that tomorrow’s selves can work more easily.

A reader asked me to discuss refactoring in light of Martin Fowler’s Refactoring: Improving the Design of Existing Code. I’m genuinely nervous about it. I bought that book ages and ages ago; it still sits on a bookshelf in my home in Beijing. Back then my understanding of systems wasn’t deep enough, so many of the ideas never truly sank in — I can’t claim I ever really “read” it. These days I don’t have the book at hand, so I can’t talk about refactoring by following along its pages. Martin Fowler’s books and blog are both worth reading, and this book is presumably no exception (I can’t speak to the translation — I honestly have no memory of it). Go buy a copy and leaf through it.

Here I’ll share my own views on refactoring from my day-to-day work.

The Way (道)

Automated Tests

The single most important precondition for refactoring code is: automated tests. The Master, standing by the river, said:

All refactoring without automated tests is plain hooliganism.

What is code written for? So that the resulting system meets its functional requirements. What is refactoring for? So that the code meeting those requirements can become, in the spirit of the Olympics, higher in quality, better in performance, faster — so that future work is lighter and the code looks more beautiful. If you refactor and break basic functionality, what good are pretty code and high performance?

So how do you guarantee that refactoring doesn’t break existing functionality? Answer: tests. Whether unit tests, functional tests, integration tests, or bilibili tests — you need to test every way you can. Refactoring happens at individual “points” (cells), so you need unit tests; it also happens at “aspects” (organs), so you need functional tests; when an “aspect” changes drastically (an organ transplant), you need integration tests… Whether the relevant tests exist determines whether you can refactor; how long the tests take determines whether you will refactor, and at what frequency. If refactoring ten lines of code means spending an hour to run the unit tests once, you’ll either not refactor at all, or refactor without testing.

Good refactoring happens all the time while you build the system, not when a problem erupts or your boss demands it. If the tests tell you the result the instant you finish refactoring, you’ll have the confidence to refactor more, until it becomes part of your working life.

You may object: what kind of unit test takes an hour to run? Answer: a manual one. Which is why the precondition is “automated tests,” not merely “tests.” Without automated tests (hereafter just “tests”), talking about refactoring is nonsense. If the area you want to refactor has poor test coverage, first raise the coverage. If there are no test cases at all, master that basic skill before you talk about refactoring.

Refactor All the Time

I’ve already said when the best time to refactor is: while writing each line of code, not when your pants are on fire. So under what circumstances should you refactor? There is a saying: bad code smells — the following uncomfortable situations are hints that your code needs refactoring:

1. When you write a piece of code and have to copy-paste from somewhere else.

Obviously this violates DRY (Don’t Repeat Yourself). If a piece of code (or documentation, tests, comments) is going to be duplicated, its logic should be extracted and stand on its own. This is about the most basic refactoring practice there is. And yet, from small startups to giant corporations, this is one of the most severe problems in nearly every system. At a company I used to work for, I maintained a C function over 5,000 lines long. The copy-and-paste inside its layer upon layer of nested if-elses was jaw-dropping; adding a bit of logic meant checking seven or eight places to see whether the same logic was needed there too. It deserves a place in a textbook as a classic negative example.

Today, thanks to the tireless efforts of masters like the GoF, Martin Fowler, Kent Beck, and Yukihiro Matsumoto, and to programmers like yours truly waving the flag from a corner, DRY code is taken more and more seriously. That’s good. But what about test code? Documentation? Comments? Shouldn’t we take those seriously too? I once took over the tests for an API system. Every test did the same thing: send one (or more) requests to a mock API server and verify whether the response was correct. Dozens of test cases, hundreds of lines of code, could have been expressed with a dozen lines of shared code plus the following declarative abstraction:

- test: 'test description'
  endpoint: '/path/to/your/api'
  expected:
    code: 200
    type: type of the response body
    body:
      # each value of the body we want to verify
      '0.type': 'scheme'
      '0.children.#empty': false
  # which group of this test belongs to
  tag: ['smoke']

Don’t underestimate DRY in tests and DRY in documentation; they matter just as much — if not more. If documentation gets copied and pasted around, a change in one place may never reach all its copies, misleading the people who read it. If test code isn’t DRY, then once it grows large enough, maintaining and updating it becomes a real headache.

2. When you’re modifying existing code to add a feature and keep feeling that something about that code is off.

Maybe the logic is too convoluted, too complex, too hard to follow; too many loops, too many branches, too much state. Code like that is practically on its knees begging to be refactored; there’d be no excuse not to.

3. When calling existing code (a function, a class), you have to read the callee’s implementation before you can figure out how to call it.

Either the interface is badly defined — a function with a dozen-plus parameters, say — or the documentation is badly written, with critical functions lacking adequate explanation of their interface. If the previous points are pure code refactoring, this one is refactoring the user experience. What is a programmer’s code? It’s a product one programmer crafts for another; a function’s (or class’s) signature, together with its explanation, is that product’s UI. If you open WeChat and some button does God-knows-what, always behaving differently from what you expect, don’t you want to jump up and curse like a jerk? By the same token, programmers are human. However much the job has sanded them down to be “gentle, kind, courteous, modest, and yielding,” the sight of an incomprehensible interface will turn them into a foul-mouthed, WTF-spewing jerk.

4. When writing one piece of code forces you to change a lot of other code.

When this happens, the code smells truly foul: it means not only the code itself is flawed, but the design — even the architecture — of the surrounding code is deeply problematic. A programmer without solid fundamentals will find such refactoring quite a struggle.

Strict Self-Discipline and Discipline from Others

Any software project of even modest size is built by several people together. When working with others, hold two beliefs: human nature is lazy — given a shortcut, nobody will dutifully walk the broad road; and people are also swayed by role models and community. If the existing codebase has established a good atmosphere, newcomers feel an urgency to fit into the existing system.

Open-source projects have a lot to teach us here. Look at the famous ones: many contributors don’t necessarily keep such good habits in their day jobs, but in the open-source project, the project’s own checks and the community’s pressure keep them disciplined.

We can guide these two aspects of human nature as follows.

At the very start of a project, set up a sufficiently high bar for code check-in. Lint, build, and test — none may be skipped. These are the linting rules I set for my team at the company (yes, we use Node.js):

{
  "extends": "airbnb/base",
  "rules": {
    "no-console": 2,
    "max-statements": [2, 50],
    "max-depth": [2, 5],
    "max-nested-callbacks": [2, 3],
    "max-params": [2, 5],
    "complexity": [2, 10],
    "max-len": [2, 120, 4]
  }
}

Beyond the basics of Airbnb’s JavaScript style, I add these requirements:

• No form of console.xxx may appear in the code. To that end, I went so far as to define a print function for CLI use. This forces programmers to think hard about how to log and which log level is appropriate.
• A function may be at most 50 lines. So what if it’s longer? Split it or trim it.
• A function may nest at most 5 levels deep. Multiple for loops, deep if-elses — these are the root of all evil. Exceed this limit and you must split the function, or go functional: map/filter/reduce.
• A function may have at most 3 levels of callbacks. This forces programmers away from callback misuse and toward Promise.
• A function may take at most 5 parameters. A function with too many parameters is basically trying to knead too many concerns into one.
• A function’s complexity may not exceed 10. All your branches, loops, callbacks and so on, added together, must not exceed 10 within a single function (note: not nested).

These requirements are outrageously strict. Complexity in particular — I often accidentally exceed the limit. Yet it forces me to think harder about the code I’m about to write, and to turn more code logic into data. Turning code into data is a crucial step in abstract thinking (like the test fixture above): it further decouples code from code and links things together with data — a set of rules.

Besides lint, mandatory test cases matter too. I put these tasks in the pre-commit hook. To check in code, you first pass the lint / build / test gate. Sure, a git pre-commit hook can be skipped, but anyone who dares will face strict code review and CI. Any violation gets the rod.

All of the above deals with the laziness in human nature. First comes the big stick.

Next you need role models, or a community atmosphere, to guide behavior. For projects I start, I write the initial code myself, making sure every line is clear and readable, every function carefully considered, every interface friendly documented, every critical function thoroughly tested — and then I gatekeep strictly in code review. That way, as more people join, they have templates to follow and experience to lean on ahead of them and the lash behind them, and they naturally write reasonably beautiful code. Xunzi said:

Ganjiang and Moye (sigh — of all the famous swords, these two are the only ones I remember) and the like were the finest swords of antiquity — yet without grinding they cannot be sharp, and without human effort they cannot cut… So a person, though of fine nature and discerning mind, must seek a worthy teacher to serve and choose good friends to befriend.

This is the power of role models and environment. With that foundation laid, every “well done” in review and every LGTM (look good to merge) on a pull request is a carrot, letting engineers taste the sweetness of writing excellent code.

None of this looks like it has anything to do with refactoring, yet at every turn it demands that programmers refactor their code to meet a higher standard. Trust me: even if people are uncomfortable at first, once they gradually build confidence they’ll pursue higher-quality code on their own.

The Technique (术)

As for the technique of refactoring — Martin Fowler’s book presumably covers it all, so I’ll skip it. Anything I said would just be picking up the crumbs of various programming patterns and paradigms. Besides, paradigms differ: refactoring classes in object-oriented programming is not the same as refactoring functions in functional programming. And languages differ wildly in their means of refactoring — Elixir (pattern matching, macros), JavaScript (closures, FP), and C++ (OOP) each demand their own approach.