Slashdot Mirror


Working Effectively with Legacy Code

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. 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 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.

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.

18 of 208 comments (clear)

  1. Not needed by eln · · Score: 5, Funny

    This book is a waste of paper. Everyone knows the proper way to deal with legacy code:

    1.) Spend 2 weeks looking at code you don't understand.
    2.) Loudly complain about the poor quality of the code, particularly algorithms that you don't understand.
    3.) Make derogatory comments about the previous developers. Be sure to paint them as monosyllabic imbeciles who probably got dropped on their heads multiple times as children.
    4.) Make minor changes to the code. If they blow up in your face, blame the previous developers for their poor grasp of basic programming practices. Make references to the previous programmers' relationship with their mothers.
    5.) Delete the whole thing and start from scratch.
    6.) 18 months of fumbling around later, realize that the previous code may have been better than you gave it credit for.
    7.) Deny this.
    8.) Release cobbled-together mess that lacks half the features of the previous codebase and features twice the bugs.
    9.) Get job elsewhere.
    10.) Company hires new programmer who starts the process over at step 1.

    1. Re:Not needed by jellomizer · · Score: 4, Insightful

      You work at GE right?

      --
      If something is so important that you feel the need to post it on the internet... It probably isn't that important.
    2. Re:Not needed by jellomizer · · Score: 4, 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.
       

      --
      If something is so important that you feel the need to post it on the internet... It probably isn't that important.
    3. Re:Not needed by Greyfox · · Score: 4, Insightful

      Good management usually reacts to calls to rewrite with skepticism. Usually (but not always) this is a good thing.

      --

      I'm trying to teach myself to set people on fire with my mind... Is it hot in here?

  2. Whatever by Anonymous Coward · · Score: 3, Informative

    Buy Martin Fowler's Refactoring instead.

  3. Not exactly... by Kindaian · · 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... ;)

  4. I Got Your Legacy System Right Here by cbowland · · Score: 4, Insightful

    A legacy system is anything that is in production RIGHT NOW. My coding philosophy has always been "building tomorrow's legacy systems today."

    --

    Give a man a fish and he will eat for a day.
    Teach him to eat and he will fish forever.

  5. If it compiles... by blindd0t · · Score: 4, Funny

    ...release it. ;-)

    1. Re:If it compiles... by morgan_greywolf · · Score: 3, Funny

      Damn it! I told you not to go around giving out our release process! That's company-proprietary information! *throws chair* You're fired! I'm gonna fscking KILL blindd0t!

      -- Steve Ballmer

  6. what can tests really do... by drDugan · · 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.

  7. Building the Legacy Systems of Tomorrow by dwheeler · · Score: 3, Insightful

    I have this bumpersticker posted on my office wall: "Building the Legacy Systems of Tomorrow". I'm not sure who created that phrasing - or the bumper sticker - but I like it.

    In short: if it runs, it's a legacy system.

    --
    - David A. Wheeler (see my Secure Programming HOWTO)
  8. Wait a second by Anonymous Coward · · Score: 3, Funny

    Did you just refactor his review? BRILLIANT!

  9. encapsulation by Dan667 · · Score: 4, Informative

    The most successful strategy I have had for legacy code that I have inherited is encapsulation of the old code into a new framework. I first attempt to build a black box wrapper with an API for what ever the legacy code did (wrap 5000 line loops, etc). Then as I can or need to change it, I take the black box and break it into proper libraries or readable functions (or start over). Have been able to do this for some really large bases of code and have a working system while I re-factored the mess a little at a time.

  10. Combinatorial Explosion by natoochtoniket · · Score: 3, 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.

  11. TDD is a waste of time and money. by TheGeneration · · Score: 3, Interesting

    Not once, EVER have I worked on code where the unit tests broke because of a bug or a mistake. Instead the unit tests break because the new code has something the test didn't anticipate. This is especialy the case in Easy Mock and TestNG.

    Maybe if you're working in a system with complex interdependence patterns, but generally it's a waste of time and money and just a management level masturbatory exercise foisted on engineering.
    ("I'm a super CTO of Cisco System! I'm going to force unit testing across the board, even where it doesn't makes sense! I'm going to be super ISO certified and John Chambers is going to lick my balls after his retirement when I become CEO!")

    --


    The Generation
    I'd say something witty here, but I'm not that bright.
  12. Legacy Code by Orion+Blastar · · Score: 3, Informative

    I have had good luck with legacy code, here is what I do:

    #1 Figure out what the code does and document it with comments and write a document on it.

    #2 Identify variables and objects and what they are used for and any naming convention the code may use.

    #3 You need to stick to the original style of writing or rewrite parts of it into your style if it can give a performance boost or make it more stable.

    #4 Try to find programming errors and things that do not make sense and rewrite them so that they make sense. Do error trapping and check for nulls and letters entered into number variables and all other sorts of things most legacy programmers overlook.

    #5 Work to make the code stable and not crash and run faster before you start adding new features to it. Users don't want to wait 15 minutes to do a report and then have the program crash after their wait.

    #6 Work with the help desk to identify the most serious problems that users complain about the legacy code. Make it a "wish list" and then fix each complaint as you have time to do.

    #7 Get direction from your managers, tell them what you are trying to do and any problems you have. You need to work as a team with other developers, the help desk, managers, and users to work out the issues with legacy code. Explain to them when you need more time and cannot make the schedule they gave. Make a deal with them to release a stable version but lacking features that might take more time than they thought to do. Tell the users you had to no add in those features to meet a deadline or ask them if they want to wait until you figure out how to add in those features.

    #8 Play Sherlock Holmes and read books or Internet web sites on the language and technology used with the legacy code. Search knowledge bases and blogs and forums for answers to solutions, sometimes someone else figured out what you are trying to solve. If not ask on a forum or blog or web site and see who answers. Many of my answers got that way from the Internet on legacy code, but management didn't understand why I spent so much time on the Internet. It was because they wouldn't buy me the books I needed and I had no documentation or anything to work with except for pure code with no comments and all with serious problems. Sometimes I had to spend 5 hours a day researching on the Internet and 3 or 4 hours coding, but in doing so I saved months of work, but management didn't understand that each web site I went to was work related and I looked at the design of sample code even HTML code to get ideas on how to solve the legacy code problems. Sometimes you have to call up a help desk of a vendor to get answers as well, but they docked me for long distance calls to Canada where Crystal Reports and Segate/Business Objects had their headquarters. Fixing Crystal Report errors would make me spend 5 hours a day on the Internet just to figure out what caused double lines in a report and why only certain users got it and not others.

    #9 When in doubt ask for help. Sometimes another pair of eyes can spot errors and mistakes that you cannot see. Diversity is a good thing with team members. Form a dream team of programmers of different backgrounds for best results.

    #10 When in danger, when in doubt, don't run in circles and scream and shout. Take a walk, get something to drink and relax. Take a mental health break instead of getting angry at other people for not helping you or not doing their jobs properly, they might be suffering from stress like you are and you don't know it. Be positive, not negative.

    --
    Remember, Slashdot does not have a -1 disagree moderation, and no, troll, flamebait, and overrated are not substitutes.
  13. Great book by IcyHando'Death · · 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!

  14. Re:testing is a waste of time by Surt · · Score: 3, Informative

    Either you misunderstood my post, or you have a fundamental misunderstanding of the halting problem.

    It says that no algorithm can decide in general whether or not a given program / input pair will halt or not. The emphasis on the in general is the key.

    It is actually trivial to do in many, many specific cases.

    http://en.wikipedia.org/wiki/Halting_problem

    In particular, take note of the second sentence:

    Alan Turing proved in 1936 that a general algorithm to solve the halting problem for all possible program-input pairs cannot exist.

    (emphasis mine)

    --
    "Who is the Journal of Quantum Physics going to believe?" --Stephen Hawking