Working Effectively with Legacy Code

Author: Michael C. Feathers
4.5
All Stack Overflow 260
This Year Stack Overflow 5
This Month Hacker News 1

About This Book

Is your code easy to change? Can you get nearly instantaneous feedback when you do change it? Do you understand it? If the answer to any of these questions is no, you have legacy code, and it is draining time and money away from your development efforts.

 

In this book, Michael Feathers offers start-to-finish strategies for working more effectively with large, untested legacy code bases. This book draws on material Michael created for his renowned Object Mentor seminars: techniques Michael has used in mentoring to help hundreds of developers, technical managers, and testers bring their legacy systems under control.

 

 

The topics covered include

  • Understanding the mechanics of software change: adding features, fixing bugs, improving design, optimizing performance
  • Getting legacy code into a test harness
  • Writing tests that protect you against introducing new problems
  • Techniques that can be used with any language or platform—with examples in Java, C++, C, and C#
  • Accurately identifying where code changes need to be made
  • Coping with legacy systems that aren't object-oriented
  • Handling applications that don't seem to have any structure

This book also includes a catalog of twenty-four dependency-breaking techniques that help you work with program elements in isolation and make safer changes.

Comments

by anonymous   2018-04-02
Why not improve the Grails Service you have? The problems you mentioned are not inherent in grails/groovy and in solving them you will probably uncover some aspects of the project that you don't know of now. If the tests are slow, what can you do to speed them up? What would a good API be for mocking the results for the tests that make http calls? I would recommend the book [Working Effectively with Legacy Code](https://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052), while it is quite old the advice in it is still mostly good.
by anonymous   2018-03-19

Indeed, there's code duplicated here, so it's a good idea to try to eliminate it.

You could do:

function askQuestion($data, $question){
  if($data !='') {
     echo "<span class='question'>$question</span><span class='answer'>".$data.'</span><br />';
   }
}

and use it like:

foreach ( $answers as $a )   {
  askQuestion($a->q2, "Question title?");
  askQuestion($a->q3, "Question label 2?");
  // and so on

And, as a general rule of thumb: when you're about to refactore code, don't forget to put in place non regression tests first (because it would be too bad to break the code when we're trying to improve it).

Last advice: if you have to work with legacy codebases on a regular basis, you might to read Working Effectively with legacy code, which gives a lot of practical tips to turn such codebases into stuff easier to work with.

by anonymous   2018-03-12
I'm not sure what you mean by sending events. I would recommend reading the book https://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052/ That book teaches you how to find the seams in your code where you can isolate behavior and just test what's important. The fact that your code is in private methods is good, but what about extracting those private methods into their own class and passing in all needed info in a public method that can be tested?
by bloat   2018-01-28
I would try and clean up the bits I was working on.

This is a good book on the topic refactoring a large code base with no tests.

https://www.amazon.co.uk/Working-Effectively-Legacy-Michael-...

by PaulHoule   2017-11-25
You're definitely right that unit tests are a part of the solution.

https://www.amazon.com/Working-Effectively-Legacy-Michael-Fe...

can be read in a few different registers (making a case for what unit tests should be in a greenfield system, why and how to backfit unit tests into a legacy system) but it makes that case pretty strongly. It can seem overwhelming to get unit tests into a legacy system but the reward is large.

I remember working on a system that was absolutely awful but was salvageable because it had unit tests!

Also generally getting control of the build procedure is key to the scheduling issue -- I have seen many new project where a team of people work on something and think all of the parts are good to go, but you find there is another six months of integration work, installer engineering, and other things you need to do ship a product. Automation, documentation, simplification are all bits of the puzzle, but if you want agility, you need to know how to go from source code to a product, and not every team does.

by anonymous   2017-10-08
@gdbj good luck in any case, we've all been there! Strongly recommended reading: [Working Effectively With Legacy code by Michael Feathers](https://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052)
by PaulHoule   2017-10-06
If you have to write mocks in the native language, mocks will probably drive you insane.

Tools like mockito can make a big difference.

I worked on a project which was terribly conceived, specified, and implemented. My boss said that they shouldn't even have started it and shouldn't have hired the guy who wrote it! Because it had tests, however, it was salvageable, and I was able to get it into production.

This book

https://www.amazon.com/Working-Effectively-Legacy-Michael-Fe...

makes the case that unit tests should always run quickly, not depend on external dependencies, etc.

I do think a fast test suite is important, but there are some kinds of slower tests that can have a transformative impact on development:

* I wrote a "super hammer" test that smokes out a concurrent system for race conditions. It took a minute to run, but after that, I always knew that a critical part of the system did not have races (or if they did, they were hard to find)

* I wrote a test suite for a lightweight ORM system in PHP that would do real database queries. When the app was broken by an upgrade to MySQL, I had it working again in 20 minutes. When I wanted to use the same framework with MS SQL Server, it took about as long to port it.

* For deployment it helps to have an automated "smoke test" that will make sure that the most common failure modes didn't happen.

That said, TDD is most successful when you are in control of the system. In writing GUI code often the main uncertainty I've seen is mistrust of the underlying platform (today that could be, "Does it work in Safari?")

When it comes to servers and stuff, there is the issue of "can you make a test reproducible". For instance you might be able to make a "database" or "schema" inside a database with a random name and do all your stuff there. Or maybe you can spin one up in the cloud, or use Docker or something like that. It doesn't matter exactly how you do it, but you don't want to be the guy who nukes the production database (or a another developer's or testers database) because the build process has integration tests that use the same connection info as them.

https://www.amazon.com/Working-Effectively-Legacy-Michael-Fe...

by anonymous   2017-09-24
This may not be a nice experience. Consider reading "Working Effectively With Legacy Code" (https://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052)
by anonymous   2017-08-20

About testability

Due to the use of singletons and static classes MyViewModel isn't testable. Unit testing is about isolation. If you want to unit test some class (for example, MyViewModel) you need to be able to substitute its dependencies by test double (usually stub or mock). This ability comes only when you provide seams in your code. One of the best techniques used to provide seams is Dependency Injection. The best resource for learning DI is this book from Mark Seemann (Dependency Injection in .NET).

You can't easily substitute calls of static members. So if you use many static members then your design isn't perfect.

Of course, you can use unconstrained isolation framework such as Typemock Isolator, JustMock or Microsoft Fakes to fake static method calls but it costs money and it doesn't push you to better design. These frameworks are great for creating test harness for legacy code.

About design

  1. Constructor of MyViewModel is doing too much. Constructors should be simple.
  2. If the dependecy is null then constructor must throw ArgumentNullException but not silently log the error. Throwing an exception is a clear indication that your object isn't usable.

About testing framework

You can use any unit testing framework you like. Even MSTest, but personally I don't recommend it. NUnit and xUnit.net are MUCH better.

Further reading

  1. Mark Seeman - Dependency Injection in .NET
  2. Roy Osherove - The Art of Unit Testing (2nd Edition)
  3. Michael Feathers - Working Effectively with Legacy Code
  4. Gerard Meszaros - xUnit Test Patterns

Sample (using MvvmLight, NUnit and NSubstitute)

public class ViewModel : ViewModelBase
{
    public ViewModel(IMessenger messenger)
    {
        if (messenger == null)
            throw new ArgumentNullException("messenger");

        MessengerInstance = messenger;
    }

    public void SendMessage()
    {
        MessengerInstance.Send(Messages.SomeMessage);
    }
}

public static class Messages
{
    public static readonly string SomeMessage = "SomeMessage";
}

public class ViewModelTests
{
    private static ViewModel CreateViewModel(IMessenger messenger = null)
    {
        return new ViewModel(messenger ?? Substitute.For<IMessenger>());
    }

    [Test]
    public void Constructor_WithNullMessenger_ExpectedThrowsArgumentNullException()
    {
        var exception = Assert.Throws<ArgumentNullException>(() => new ViewModel(null));
        Assert.AreEqual("messenger", exception.ParamName);
    }

    [Test]
    public void SendMessage_ExpectedSendSomeMessageThroughMessenger()
    {
        // Arrange
        var messengerMock = Substitute.For<IMessenger>();
        var viewModel = CreateViewModel(messengerMock);

        // Act
        viewModel.SendMessage();

        // Assert
        messengerMock.Received().Send(Messages.SomeMessage);
    }
}
by Ruben Bartelink   2017-08-20

Other hanselminutes episodes on testing:

Other podcasts:

Other questions like this:

Blog posts:

I know you didn't ask for books but... Can I also mention that Beck's TDD book is a must read, even though it may seem like a dated beginner book on first flick through (and Working Effectively with Legacy Code by Michael C. Feathers of course is the bible). Also, I'd append Martin(& Martin)'s Agile Principles, Patterns & Techniques as really helping in this regard. In this space (concise/distilled info on testing) also is the excellent Foundations of programming ebook. Goob books on testing I've read are The Art of Unit Testing and xUnit Test Patterns. The latter is an important antidote to the first as it is much more measured than Roy's book is very opinionated and offers a lot of unqualified 'facts' without properly going through the various options. Definitely recommend reading both books though. AOUT is very readable and gets you thinking, though it chooses specific [debatable] technologies; xUTP is in depth and neutral and really helps solidify your understanding. I read Pragmatic Unit Testing in C# with NUnit afterwards. It's good and balanced though slightly dated (it mentions RhinoMocks as a sidebar and doesnt mention Moq) - even if nothing is actually incorrect. An updated version of it would be a hands-down recommendation.

More recently I've re-read the Feathers book, which is timeless to a degree and covers important ground. However it's a more 'how, for 50 different wheres' in nature. It's definitely a must read though.

Most recently, I'm reading the excellent Growing Object-Oriented Software, Guided by Tests by Steve Freeman and Nat Pryce. I can't recommend it highly enough - it really ties everything together from big to small in terms of where TDD fits, and various levels of testing within a software architecture. While I'm throwing the kitchen sink in, Evans's DDD book is important too in terms of seeing the value of building things incrementally with maniacal refactoring in order to end up in a better place.