How do you test/change untested and untestable code?

Question

Lately I had to change some code on older systems where not all of the code has unit tests.
Before making the changes I want to write tests, but each class created a lot of dependencies and other anti-patterns which made testing quite hard.
Obviously, I wanted to refactor the code to make it easier to test, write the tests and then change it.
Is this the way you'd do it? Or would you spend a lot of time writing the hard-to-write tests that would be mostly removed after the refactoring will be completed?

Answer 1

First of all, here's a great article with tips on unit testing. Secondly, I found a great way to avoid making tons of changes in old code is to just refactor it a little until you can test it. One easy way to do this is to make private members protected, and then override the protected field.

For example, let's say you have a class that loads some stuff from the database during the constructor. In this case, you can't just override a protected method, but you can extract the DB logic to a protected field and then override it in the test.

public class MyClass {
    public MyClass() {
        // undesirable DB logic
    }
}

becomes

public class MyClass {
    public MyClass() {
        loadFromDB();
    }

    protected void loadFromDB() {
        // undesirable DB logic
    }
}

and then your test looks something like this:

public class MyClassTest {
    public void testSomething() {
        MyClass myClass = new MyClassWrapper();
        // test it
    }

    private static class MyClassWrapper extends MyClass {
        @Override
        protected void loadFromDB() {
            // some mock logic
        }
    }
}

This is somewhat of a bad example, because you could use DBUnit in this case, but I actually did this in a similar case recently because I wanted to test some functionality totally unrelated to the data being loaded, so it was very effective. I've also found such exposing of members to be useful in other similar cases where I need to get rid of some dependency that has been in a class for a long time.

I would recommend against this solution if you are writing a framework though, unless you really don't mind exposing the members to users of your framework.

It's a bit of a hack, but I've found it quite useful.