I've seen the book Working Effectively with Legacy Code recommended a few times. What are the key points of this book?
Is there much more to dealing with legacy code than adding unit/integration tests and then refactoring?
Software Engineering Stack Exchange is a question and answer site for professionals, academics, and students working within the systems development life cycle. It only takes a minute to sign up.Sign up to join this community
The key problem with legacy code is that it has no tests. So you need to add some (and then more...).
This in itself would take a lot of work, as @mattnz noted. But the special problem of legacy code is that it was never designed to be testable. So typically it is a huge convoluted mess of spaghetti code, where it is very difficult or downright impossible to isolate small parts to be unit tested. So before unit testing, you need to refactor the code to make it more testable.
However, in order to refactor safely, you must have unit tests to verify that you haven't broken anything with your changes... This is the catch 22 of legacy code.
The book teaches you how to break out of this catch by making the absolute minimal, safest changes to the code just to enable the first unit tests. These aren't meant to make the design nicer - only to enable unit tests. In fact, sometimes they do make the design uglier or more complex. However, they allow you to write tests - and once you have unit tests in place, you are free to make the design better.
There are lots of tricks to make the code testable - some are sort of obvious, some are not at all. There are methods I would have never thought about myself, without reading the book. But what is even more important is that Feathers explains what precisely makes a code unit testable. You need to cut dependencies and introduce barriers into your code, but for two distinct reasons:
Cutting dependencies safely can be tricky. Introducing interfaces, mocks and Dependency Injection is clean and nice as a goal, just not necessarily safe to do at this point. So sometimes we have to resort to subclassing the class under test in order to override some method which would normally e.g. start a direct request to a DB. Other times, we might even need to replace a dependency class/jar with a fake one in the test environment...
To me, the most important concept brought in by Feathers is seams. A seam is a place in the code where you can change the behaviour of your program without modifying the code itself. Building seams into your code enables separating the piece of code under test, but it also enables you to sense the behaviour of the code under test even when it is difficult or impossible to do directly (e.g. because the call makes changes in another object or subsystem, whose state is not possible to query directly from within the test method).
This knowledge allows you to notice the seeds of testability in the nastiest heap of code, and find the minimal, least disruptive, safest changes to get there. In other words, to avoid making "obvious" refactorings which have a risk of breaking the code without you noticing - because you don't yet have the unit tests to detect that.
I work on a code base of millions of lines of code, some dating back to the 1980's. It's just software, so it's just a matter of writing a few unit tests, so you can go and just refactor it, and make it so much better.
The key word here is just - it's a four-letter word that does not belong in any programmer's vocabulary, let alone one who is working on legacy systems.
How long do you think it takes to write a unit test, to test one hour worth of development effort? For discussion sake, let's say another hour.
How much time is invested in that million-line, 20-year-old legacy system? Let's say, 20 developers for 20 years times 2000 hours / annum (they worked pretty hard). Let's now pick a number - you have new computers and new tools, and you are so much more clever than the guys who wrote this piece of $%^^ in the first place - let's say you are worth 10 of them. Have you got 40 man years, well, have you...?
So the answer to your question is there is much much more. For instance, that routine that's 1000 lines (I have a few that are over 5000), it is overly complex and is a piece of spaghetti. It would only (yet another four letter word) take a couple of days to re-factor it into a few 100 line routines and a few more 20 lines helpers, right? WRONG. Hidden in those 1000 lines is 100 bug fixes, each one an undocumented user requirement or an obscure edge case. It's 1000 lines because the original 100-line routine did not do the job.
You need to work with the mind set "if it ain't broke, don't fix it". When it is broke, you need to be very careful when you fix it - as you make it better, that you don't accidentally change something else. Note that "broke" may include code that is unmaintainable, but working correctly, that depends on the system and its use. Ask "what happens if I screw this up and make it worse", because one day you will, and you will have to tell the bosses' boss why you chose to do that.
These systems can always be made better. You will have a budget to work to, a timeline, whatever. If you don't - go and make one. Stop making it better when the money/time has run out. Add a feature, give yourself time to make it a little better. Fix a bug - again, spend a little extra time and make it better. Never deliver it worse than it was when you started.
There are two key points to take away from the book.
As other responders have pointed out, trying to pre-emptively update your existing legacy code is a fool's errand. Instead, whenever you have to make a change to legacy code (for a new feature or a bug fix), take the time to remove its legacy status.