Working Effectively with Legacy Code 208
Merlin42 writes "I recently took a Test-Driven-Development (TDD) training course and the teacher recommended that I read "Working Effectively with Legacy Code" by Michael Feathers. First things first, a note about the title. Feathers defines "Legacy Code" a bit different than you may expect, especially if you are not into the XP/Agile/TDD world. I have heard (and used) a number of definitions for "legacy code" over the years. Most of these definitions have to do with code that is old, inherited, difficult to maintain, or interfaces with other 'legacy' hardware/software. Feathers' definition is 'code without tests.' For those not into TDD this may seem odd, but in the TDD world, tests are what make code easy to maintain. When good unit tests are in place, then code can be changed at will and the tests will tell automatically you if you broke anything." Read on for the rest of Kevin's review.
Overall this is definitely an interesting read, and useful to anyone who has ever yelled "FSCKing LEGACY code!" It will be most useful to someone who already has some appreciation for TDD and wants to use it to 'pay down the technical debt' in a legacy code project. In my opinion adding unit tests (a sort of retroactive TDD) is the best ... err ... most effective approach for getting a legacy code project into a more malleable state.| Working Effectively with Legacy Code | |
| author | Michael Feathers |
| pages | 456 |
| publisher | Prentice Hall |
| rating | 9/10 |
| reviewer | Kevin Fitch |
| ISBN | 978-0-13-117705-5 |
| summary | Excelent overview of how to apply TDD to an existing project |
One caveat is that most of the book is focused on working with object oriented programming languages. There is some coverage of techniques for procedural languages (mainly C), but this is not the main focus of the book. In a way this is unfortunate, since there is a lot of really useful C code out there gathering dust. But in the book he states that "the number of things you can do to introduce unit tests in procedural languages is pretty small." Unfortunately I would have to agree with him on this point.
One of the greatest things about this book is that it is written by someone who has worked with a lot of legacy code, and there are numerous real world anecdotes sprinkled throughout the text that really serve to help drive the points home. The code examples are plentiful, but not verbose. They all look like real code you might find lurking in a dark corner at work, not some fanciful made up snippet.
The high level goal of the book is show you how to write good unit tests for code that wasn't designed with unit tests in mind. The first step for writing unit tests is getting individual classes or functions into a test harness where you can apply known inputs, and check the outputs or behavior. To do this you need to break dependencies in the original code. The bulk of the book is dedicated to looking at different approaches to breaking dependencies.
Much of the book is organized like a FAQ. There are chapter titles like: "I Need to Make a Change. What Methods Should I Test?" and "My Project Is Not Object Oriented. How Do I Make Safe Changes?". This organization makes the book work a bit better as reference than as learning material. After the first few chapters there is very little flow to the book. Each chapter tends to stand as an independent look into a particular problem common in legacy code. As a result, you can read the table of contents and usually skip to a self-contained chapter that will help with the problem at hand.
The final chapter of the book is a listing of all the refactoring techniques used throughout the rest of book. So if you have a particular dependency-breaking technique in mind, you can skip straight to the description of the technique you want to use. This can be quite helpful when you need to perform a refactoring before you can get your code into a test harness. The descriptions are straightforward and provide a little checklist at the end that will help you make sure you didn't miss anything.
In conclusion I would definitely recommend this book to a colleague who is trying to introduce unit tests into code that was not designed with testing in mind. In fact I have already lent the book to several people at work, most of whom have bought their own copy.
You can purchase Working Effectively with Legacy Code from amazon.com. Slashdot welcomes readers' book reviews -- to see your own review here, read the book review guidelines, then visit the submission page.
Re:Not needed (Score:5, Insightful)
You work at GE right?
Not exactly... (Score:5, Insightful)
The simple passing of all tests doesn't necessarily means that you didn't broke anything.
It means only that you passed the tests.
If the tests don't provide coverage for ALL the business issues that the piece of software is supposed to solve, then you pass the tests, but will have no clue if you broke or not things apart.
Best approach is to evaluate current test procedures and check if they provide enough coverage for at least all user related actions and all the automated actions.
Only after you know that your testing procedures are sound, you can have that assurance... ;)
I Got Your Legacy System Right Here (Score:5, Insightful)
A legacy system is anything that is in production RIGHT NOW. My coding philosophy has always been "building tomorrow's legacy systems today."
Re:testing is a waste of time (Score:1, Insightful)
Test driven design works if the tests are complete. Though I will admit I have rarely seen a complete set of tests; and you still need to some old fashion testing (integration testing, real world monkey testing, etc...).
Honestly I think if you're working with "legacy" code, most companies will view the time needed to get add tests as a waste of time.
TDD seems to work better with new projects where you can plan for the testing framework.
what can tests really do... (Score:5, Insightful)
I push back on this mentality each time I see it from the agile crowd: (FTA/review)
"When good unit tests are in place, then code can be changed at will and the tests will tell automatically you if you broke anything."
No. (testing FTW and all, but lets get real)
Tests are *helpful*. Multi-user development beyond 2 people accelerates with good tests. Maintenance long term is easier with tests. Changes happen faster and are more robust with good tests. However, tests are extremely difficult to write well and almost impossible that cover all the possibilities for future changes while also telling future programmers automatically when something doesn't work. I think that the best one could say is this:
When a comprehensive set of great unit tests are in place, then code can be changed at will and the tests will help the programmer understand if they broke anything. Test will often tell you automatically about things that are obvious, and usually would be seen with the most basic release testing. The art of writing good tests is understanding the subtle points of how your code functions and the pitfalls future developers may trip over when they extend what you did.
Re:Not needed (Score:5, Insightful)
I have goten you sarcasm, however I feel some people may miss it, so I will comment on these ideas as if you were serious, as this is actually more like real life then most people want to admit.
1. Trying to analysise the code is a lot of extra work not needed as you can take it for granted that it works correcly and you just need to focus on what doesn't. For most apps a quick search for the button or menu item that is causing the problem will allow you to trace you way to the module and the area that needs to be fixed.
2., 3. and part of 4. Remember people are people. I bet you even write some bad code from time to time. Either you are really tired, under a deadline, or had to work around an other bug that may have been long since fixed, or make the code so optimized that it is unmaintainable. We all do it, most of us won't admit it. So remember that when you are about to critize someones elses code. As well you need to reference the code quality to the time it was made. Look at code in the 1980's it was usually written by people with out Computer Science Degrees, so there will be Goto and the like.
4. Making minor changes when possible is a good method. However if it blows up then you will need to make a larger change... For legacy apps your job isn't as much fixing a bug, but the user of the application has a different process that you need to adjust the computer to account for. The process may have worked for 20 years, and it worked. But it changed sometimes you can get away with a little tweak but sometimes it requires more.
5., 6. Starting from scratch could kill the company. Or be way to expensive. It has been working for 20 years and just needs some minor tweaks, yes maintaining it takes a bit more work then before but it could cost millions (not just programming time, but change management, training, research, bug fixes, missed area....) vs. Paying some guys $100k a year (taking decades to recover the cost of the inital effort)
7. If you admit to the failure you may be able to get the legacy back and running, you may still have a job, although you may get some angry bosses for a while. However you made a mistake it is better to admit it then go down the path of distruction.
8. If you did go threw the full rewrite process you should have put more effors in specing it out, and giving a clearer quote. And accounted for bugs to be fixed.
9. If you messed up to much, sometimes getting a new job is not that easy. Your reputation can spread.
10. Managers should have learned from the mistake and not allowed the new developer to do the same things.
Re:Not needed (Score:5, Insightful)
Building the Legacy Systems of Tomorrow (Score:4, Insightful)
In short: if it runs, it's a legacy system.
Re:Unit testing in Web programming (Score:3, Insightful)
Try rendering out enough hidden information in your html such that a programmatic test can drive the UI in a meaningful way (ie unique, tree-based ids for every user manipulateable element). Then testing is just a matter of recording the path of interest through the application. A script where I work might look something like:
startpage = login .ClickRevokePrivileges
AdminPage = startpage.clickAdminTab
selectedUserRow = AdminPage.UsersList.SelectRandomUser
selectedUserRow
assertFalse(selectedUserRow.hasPriviliges)
You can imagine how each of those things maps to the relevant html.
Except that (Score:1, Insightful)
Tests ARE documentation.
With the bonus property that the computer can execute them.
Re:Unit testing in Web programming (Score:1, Insightful)
Re:Legacy code (Score:3, Insightful)
The really nice to have is where the tests and the documentation are unified, making it impossible for them to diverge. That's what we've built our process around, and it works amazingly well.
Re:testing is a waste of time (Score:2, Insightful)
Re:testing is a waste of time (Score:3, Insightful)
Re:Unit testing in Web programming (Score:3, Insightful)
I'm describing a UI level unit test (a test which covers a minimal unit of UI).
If you just want to test your widgets on an individual basis, just use selenium and test pages.
That's the only two facets to web testing I can think of, so if it isn't one of those please explain what you mean. Are you thinking of load testing?
Re:Whatever (Score:3, Insightful)
No - buy BOTH books, they really do compliment each other
Combinatorial Explosion (Score:4, Insightful)
Testing cannot detect errors with probability significantly greater than zero, unless the system under test is trivially small. For a system that has N interacting features, the number of test cases that are needed to "cover" all combinations of features is O(2^N). And, that is assuming the simplest possible features that are either used or not used in each case. If any features have complicated (more than one bit) inputs, the base of that exponential complexity function increases.
While tests are helpful to detect implementation errors, test sets cannot be complete for nontrivial systems. And because testing cannot be complete, it can never provide sufficient verification. That is a basic fallacy of test-driven development, and of a-posteriori testing generally.
The least-cost way to prevent bugs that will be noticed by users is to avoid making them in the first place. Requirements and designs can be documented, checked, reviewed, communicated, and (most importantly) read and referenced during subsequent phases and iterations of the development process. Test plans and test scripts can be part of that process, but cannot replace the requirements and design phases.
Cost-driven managers don't like to hear that, though, because they think testing is cheap. Non-automated testing can often be done by cheap and easily-replaced labor. And automated testing is essentially free after the test software itself is developed and verified. (Notice, though, that developing the tests also involves requirements and designs, and increases the total amount of software that must be developed.)
So, the least cost development process involves some reasonable amount of testing, but also involves requirements and designs, and reviews at every step. The only way to defeat the combinatorial explosion is by applying heavy doses of "thinking" and "understanding". Nothing else works as well.
Re:what can tests really do... (Score:2, Insightful)
Your argument seems plausible, until you have actually seen the difference in products developed with test-driven / unit test first approaches. The benefits are not what you think they are. I would agree that unit tests are not a panacea, but disagree on
1) The are essential, not just helpful, at least, if you intend to produce software that works.
2) Unit tests do not need to be comprehensive to be useful. They don't need to be great, but great helps.
I tend to agree with you on 1) unit tests are not integration tests, and also do not replace smart human testers. 2) No TDD or agile expert would include the word "anything" in this statement "When good unit tests are in place, then code can be changed at will and the tests will tell automatically you if you broke anything"
Here is what does happen in initial development with TDD / Agile.
First, you write tests first, based on clear user stories. Then you write code to pass to pass tests (one at a time). This saves massive amounts of time: you avoid unnecessary code, over-design, and unnecessary features.
You see a side benefit quickly: In order to get code into true unit tests, each module has to do something meaningful on its own. You avoid the massive stink of excessive interdependency that paralyzes much OO code. (Much of Feather's book is about how to break those dependencies in order to test code).
Now, 1 or 2 weeks later (agile development gives you something interesting to look at in a week or two, not a month or two), it's either time to clean up the code, or you found you really misunderstood the problem. With this test coverage, it is enormously easily to re-factor.
But a warning: If you were writing integration tests (not unit tests), you'll find the tests are in the way. You make 1 change, and 10 tests fail. Done correctly, 1 change, 1 unit test fails. xUnit Test Patterns might be a good book, if you have this problem.
Is the coverage helpful or essential? Most of the code I work on has complexity rated at mind-boggling. Not only is it helpful to have the coverage, it's essential. I don't want to attempt to remember all 200 rules and exceptions when each behavior is complex enough. And don't want to wait 2 weeks for QA to finish their test pass. Good coverage means I make a change, and know immediately. I can let QA know risks that can't be covered in unit tests, and they can do some valuable work in discovery. Essential!
Re:Combinatorial Explosion (Score:3, Insightful)
And because testing cannot be complete, it can never provide sufficient verification.
That's like saying that seat belts can't save your life in every car accident so they're not worth wearing at all. Unit testing is but one tool in a developer's toolbox. It is not an all-encompassing solution to all of a project's ills.
Re:what can tests really do... (Score:3, Insightful)
As a long time Tester (10+ years) and Programmer, I'm going to go one step further:
Writing GOOD tests is HARD.
First you have to think through the use cases, business logic, etc etc etc
Then once you have the tests written, stop and think: Who is going to test that the code you just wrote (unit tests) is actually doing what you think it's doing.
I write test code for a living, and test code still scares the crap out of me for the simple reason that there's no verification happening on the test code itself apart from what the original author of the code does. Simple syntax errors in your tests may mean that what you think is being tested, is actually not being tested at all, or being verified to the wrong spec!
Unit tests are a Very Good Thing (TM)! But they are NOT the end-all-be-all of testing.
Re:Whatever (Score:2, Insightful)
Buy Martin Fowler's Refactoring [amazon.com] instead.
Remember that in Feathers' book "legacy" means not to have unit tests.
Refactoring starts with the assumption that unit tests are in place. The challenge with legacy code is that very often its current structure makes it impossible to write unit test for it. This book is about techniques of safely transforming untestable code to a form that is testable.
Only after that come actual unit tests, and after that refactoring.
All in all, the two books are complimentary.
Legacy Code (Score:3, Insightful)
I'll do you one better: Legacy code is anything developed under a different process than you're using now. If all you'll ever do is TDD, then Feathers' definition is fine. But if, like me, you've seen a dozen major development philosophies come and go and be refined over the years, you know that TDD will eventually be supplanted. The only thing that remains constant in the recognition of difficult maintenance is this: "We didn't plan to maintain it the way we're maintaining it now."
Great book (Score:4, Insightful)
I don't know how many of those leaving their pessimistic comments here have actually read this book, but I have. It's actually been on my to-do list to write a book review for Slashdot myself. Long overdue, I thought, given that the book was published in 2005. Now I'm sorry I didn't get around to it, because I think this reviewer, though positive about the book, considerably undersells it.
To those of us stuck doing active development on old, ugly code, every day can feel like we are slogging deeper and deeper into a swamp. Each time we hack in a new change, it makes us feel unclean. We are ashamed of the ugliness of the patch work we are adding to. We know programming used to be fun, but only rarely do we feel the echoes of that now. Mostly we feel dejected. And we've lost our motivation because we are not putting out code we are proud of.
If any of that rings a bell with you then grab Michael Feathers' book the next chance you get. A previous poster said something like "get Martin Fowler's Refactoring book instead", but he's entirely wrong. Not that it isn't a great book, but it won't save you. I've known about refactoring for years without being able to put any of it into practice. The prerequisite to aggressive refactoring is a good set of automated tests, and my projects have not only had no tests, but have seemed down-right untestable.
WELC is your map out of the swamp. And it's a map drawn by someone who has clearly spent a lot of time guiding others out. Feathers knows how tangled your code base is. He knows it doesn't have useful documentation or comments. He knows you are under time pressure but afraid to break funtionality you don't even know about. He has seen it all and he knows how discouraging and hopeless it looks. But he knows the way out, and he'll patiently and calmly
guide you as you break your first dependency, get your first class into a test harness or write your first test case. And before you know it, you are standing on a little patch of solid ground.
Take my advice. Get this book, read it, and put it into practice. It can change your (work) life!
Avoid the specialist books at first (Score:3, Insightful)
While I completely respect your desire to learn, I advise against rushing out to read a whole book on the subject straight away.
The reason I say this is that unit testing is really a very simple idea: you should try to design your code so that you can test each module independently; implement simple, self-contained, automated tests for each part of the interface functionality; and then run your set of tests frequently, ideally between each change you make to the code. This certainly isn't foolproof, because it relies on having a good, comprehensive set of tests and usually it's impossible to cover everything. However, you can still help yourself to find most bugs quickly, and to identify very accurately and immediately where they come from, by using a good test suite. Of course there are some useful ideas and techniques that can help you to do these things more efficiently and reliably, but the basic principle is always the same.
People write whole "frameworks" to deal with this stuff and some books discuss them, but IME these frameworks are in that category of libraries that everyone seems to write but no-one seems to use. It is often simpler and faster to write your own that fits exactly into your particular project than to learn someone else's, create a dependency on external code, and then adapt it to your specific needs anyway.
Likewise, people write whole books on software development approaches like Test Driven Development, which are heavily based on unit tests. However, while there is plentiful objective evidence that quality can be improved by using unit tests, there is precious little beyond anecdotal evidence that anything other than consultants' incomes is improved by adopting TDD and the like. (If anyone disagrees with this, please spare us all the rant unless you can cite verifiable data to support what you're going to say.)
There are some good comments on unit testing in general software development books such as Code Complete, which you might find interesting and useful. But I advise steering clear of the specialist books on frameworks and methodologies built around unit testing, at least until you have enough experience to separate the snake oil from the real oil.
Re:what can tests really do... (Score:3, Insightful)
1) The are essential, not just helpful, at least, if you intend to produce software that works.
That clearly isn't true. Arguably the most robust software in the world is produced using the Cleanroom approach, which is almost the antithesis of TDD. Of course the typical constraints for the kind of development project that uses Cleanroom are rather extreme, but that doesn't make them any less valid a counter-example.
I tend to agree with you on [...] 2) No TDD or agile expert would include the word "anything" in this statement "When good unit tests are in place, then code can be changed at will and the tests will tell automatically you if you broke anything"
One of the problems I have with these "experts" is that while they may not actually say that, a lot of them certainly give that impression to people learning from them by not explicitly correcting the over-generalisation encouraged by their general presentation, and a lie of omission is still a lie. The very existence of that claim in the Slashdot summary here is a demonstration of the way a good probability can turn into an absolute in the mind of the impressionable student.
First, you write tests first, based on clear user stories.
Which is cute, except that a lot of real world software development doesn't fall into neat little boxes like that. You can test examples, but you can't test every possible document a user might type into your word processor, every possible data set you might collect with a scientific instrument, every possible configuration designed in a CAD application, or every possible state of a game world in a MMORPG. This matters, because a new "feature" might not make any sense in isolation, only in combination with other features to give it some context, so you can't just write a single, isolated test for its behaviour. The fact that dependencies between interacting features can be significant is why unit testing alone is insufficient as an approach to quality control.
Re:I Got Your Legacy System Right Here (Score:1, Insightful)
Code is legacy the moment it is checked in. As soon as it's in source control, there is a cost to maintain it, continue testing it, and writing new code needs to take that existing code into account.
Re:Not exactly... (Score:3, Insightful)
Many systems have moving parts that are very difficult (if not impossible) to handle from a unit-testing perspective. This is particularly true for distributed systems or even multithreaded applications, where you need execution coverage, data coverage, and all combinations of all possible race conditions.
Unit testing can give you a lot of confidence for individual isolated software components, but it was never intended to address the "system as a whole".
tests make good code? (Score:2, Insightful)
"in the TDD world, tests are what make code easy to maintain. When good unit tests are in place, then code can be changed at will and the tests will tell automatically you if you broke anything."
Isn't this a rather ambitious claim? I've seen many systems with lots of tests with bugs not caught.