Review: Working Effectively with Legacy Code, by Michael Feathers

Russ Allbery eagle at eyrie.org
Sat Apr 6 20:36:32 PDT 2019


Working Effectively with Legacy Code
by Michael C. Feathers

Publisher: Prentice Hall
Copyright: 2004
Printing:  2005
ISBN:      0-13-117705-2
Format:    Trade paperback
Pages:     419

Suppose that you're familiar with the principles of good software
design, you understand the importance of breaking complex code apart
into simpler components, you know how to write good test suites, and
you can structure new code to be maintainable. However, as is so often
the case, your job is not to write green-field code. It's to add or
change some behavior of an existing system, and that existing system
was written with complete disregard to (or prior to the widespread
development of) all of those principles. How do you start?

That's the core topic of this somewhat deceptively titled book. The
title arguably overpromises, since there are many aspects of working
with legacy code that are not covered in this book (and couldn't be
covered in any one book). Feathers further narrows the topic with a
rather idiosyncratic definition of legacy: code without unit tests. The
point of the techniques discussed here is to restructure the piece of
code that you want to modify so that you can add tests (and,
specifically, unit tests; Feathers barely mentions the existence of
integration tests).

There are many perils in reading a book about programming that's this
old, but Working Effectively with Legacy Code holds up surprisingly
well, probably due to its very narrow focus. Code examples are in Java,
C++, and C, which are still among the languages that one would expect
to see in legacy code even today (although are a less comprehensive set
than they were). This book is clearly from the early, excited days of
agile and extreme programming and lacks some of the nuance that has
later developed, but the constraint of working with legacy code forces
compromises that keep it from being too "pure" of an agile diatribe,
helping its continued relevance. Feathers is obsessed with unit
testing, and I'll make some argument against that focus in a moment,
but with legacy code defined as code lacking tests, unit tests are a
reasonable place to start.

The vast majority of this book, including a long section of mechanical
recipes at the back, is devoted to specific techniques to tease apart
code where tangles prevent unit testing in isolation. Feathers's goal
is to be able to unit-test only the piece of logic that you plan on
changing, while making as few changes as possible to the rest of the
code to avoid breaking other (untested) behavior. To do this, he
describes a wide variety of refactoring techniques, some of which lead
directly to better code, and some of which lead to much worse code in
the short term but add seams where one can break code apart in a test
harness and test it in isolation. The vast majority of these techniques
involve using, and abusing, the object system of the language
(including interfaces, subclassing, overrides, making methods and data
public, and many other approaches), but he also covers techniques that
work in C such as interposition or using the preprocessor. All of the C
techniques he mentioned are ones that I've used in my own C code, and
his analysis of pluses and drawbacks seemed largely accurate in each
case, although he's far too willing to just throw a C++ compiler
willy-nilly at a large C code base.

The parts of this book that are not focused on breaking up tangled code
to allow for testing are focused on techniques for understanding that
code. Feathers is a visual thinker, so a lot of his advice uses ways of
drawing diagrams that try to capture dependencies, propagation of
values, relationships between classes, possible refactorings, and other
structural ideas of interest. I am not a very visual thinker when it
comes to code structure, so I'm not the best person to critique this
part of the book, but it seemed reasonable (and simple) to me.

Feathers mostly stops at getting code into a structure for which one
can write unit tests, following his definition of legacy code as code
without tests. Although he mentions integration testing a few times,
he's also very focused on unit tests as the gold standard of testing,
and is therefore extremely fond of fakes, mock classes, and other
approaches to test classes in isolation. This goes hand-in-hand with a
desire to make those unit tests extremely fast (and therefore extremely
simple); Feathers's ideal would be tests that could run with each
keystroke in an IDE and highlight test failures the way syntax failures
are highlighted.

This is my largest point of disagreement with this book. I understand
the appeal; when I started programming in a language that supported
easy and flexible mocking (Python with the core mock module), it was a
revelation. Those facilities, alongside interfaces and dependency
injection which make it easy to substitute fakes inside unit tests,
make it possible to test the logic of every class in isolation without
setting up a more complex testing environment. But I've subsequently
had a number of bad experiences with code that's comprehensively tested
in this fashion, which has convinced me that it's more fragile than its
advocates seem to acknowledge.

There are two, closely-related problems with this pure unit-testing
approach: one starts testing the behavior of classes in isolation
instead of the user-visible behavior of the application (which is what
actually matters), and one starts encoding the internal structure of
those classes in the test suite. The first problem, namely that each
class can be correct to its specifications in isolation but the
application as a whole could not work properly, can be caught by adding
proper integration tests. The second problem is more insidious. One of
the purposes of testing is to make refactoring and subsequent behavior
changes easier and safer, but if every jot and tittle of the internal
code structure is encoded in the test suite via all the mocks and
fakes, a simple half hour of work refactoring the code as part of
adding new functionality turns into hours of tedious work restructuring
the tests to match. The result is to paradoxically discourage
refactoring because of the painful changes then required to the tests,
defeating one of the purposes of having tests.

Feathers, as is typical of books from the early days of agile, doesn't
even mention this problem, and takes it as nearly a tautology that unit
testing and mocking out of dependencies is desirable.

One of his repeated themes is finding a way to mock out database
layers. I think this is the place where this book shows its age the
most, since that discussion focuses removing the need for a managed
test database, worries about colliding with other people's use of the
same test database, and includes other comments that assume that a
database is some external singleton outside of the development
environment of the programmer. This already wasn't the case in 2004
when one could spin up a local instance of MySQL; now, with SQLite
readily available for fast, temporary databases, it's trivial to write
tests without mocking the storage layer (as long as one is careful
about SQLite's lack of schema enforcement). For me, this tilts the
balance even farther in favor of testing user-visible functionality
across moderate-sized parts of the code base rather than isolated unit
testing. I prefer to save the mocks and fakes for dependencies that are
truly impossible to use in a test environment, such as external
hardware or APIs that call out to services outside of the scope of the
application.

I'm spending so much time on testing approaches because testing is the
hidden core of this book. What Feathers primarily means by "working
with" legacy code is testing legacy code, at least the part that you're
changing. My dubiousness about his testing focus undermined some of his
techniques for me, or at least made me wish for additional techniques
that focused on testing integrated features. But, that caveat aside,
this is a detailed look at how to untangle code and break dependencies,
useful under any testing methodology. Feathers isn't afraid to get into
the nitty-gritty and give specific examples and step-by-step
instructions for ways to untangle code patterns safely in the absence
of tests. Reading the whole book also provided me with a useful feel
for the types of options I should be considering when tackling a messy
bit of refactoring.

The audience for this book is people who already have a good
understanding of object-oriented programming techniques, but are used
to well-designed applications and want to expand their knowledge to
some of the tricks one can use to unknot gnarly problems. It's nothing
revolutionary, but if that describes you, it's a good resource.

Rating: 7 out of 10

Reviewed: 2019-04-06

-- 
Russ Allbery (eagle at eyrie.org)              <http://www.eyrie.org/~eagle/>


More information about the book-reviews mailing list