Busy Constructors (are no problem to test)

For whatever reason, I was browsing Miško Hevery's old testability blog entries and found a nice top 10 list of developer practices that make code hard to test.

Too bad I totally disagree with Item #3: Doing work in constructor. There's nothing untestable about big long constructors. No, really. Consider this horrorshow constructor, in which a global DataSource is retrieved from a mingleton and a concrete class is instantiated:

import javax.sql.DataSource;

public class UserService {

   private final DataSource dataSource;
   private final UserSettings settings;

   public UserService() {
       dataSource = DatasourceFactory.getDefault();
       settings = new UserSettingsImpl();
   }
}

Don't get me wrong, this is a horrible design. For one, retrieving the data source from a global static method is an example of a mingleton, which is global state by any other name: near impossible to inject any sort of variable behavior or polymorphism into. And instantiating a concrete implementation class (new UserSettingsImpl()) tightly couples this service to that class, another undesirable trait. This service is incredibly tightly coupled to the dependencies it requires. You'll never be able to reuse this service in another context where the datasource and user settings are different. And writing a test would be reusing this service... so what possibly makes this easy to test?

Package local constructors mixed with an Extract Parameter refactoring is what makes this easy to test. Simply create a new constructor for the object and leave out a visibility modifier. This makes the method only visible to objects in the same package (like your unit test)! Then add all the fields created in the original constructor to the parameter list. Final fields are your friend here because then the compiler will check to make sure you did this correctly. If you want to get fancy, chain the original constructor call into the new one:

public class UserService {

   private final DataSource dataSource;
   private final UserSettings settings;

   public UserService() {
       this(DatasourceFactory.getDefault(), new UserSettingsImpl());
   }

   UserService(DataSource dataSource, UserSettings settings) {
       this.dataSource = dataSource;
       this.settings = settings;
   }
}

This transformation preserves all the type signatures that were previously in the class, so all the clients that were compiled against the original code will continue to work just fine without recompiling. So even if the class you need to refactor is depended upon from everywhere, this is still a local refactoring. None of your refactor fearing coworkers even needs to know; it'll just be between us.

Did this buy us anything? Yes. All the dependencies of the class are now exposed for mocking and stubbing, without requiring changes to any calling code. Of course, your original constructor can't be tested, so if you have behavior or logic in the constructor that you need to test, then a little more creative refactoring is needed and it may not be supported with just automated IDE refactorings. Some might complain that we've modified the production code simply to enable testability... but to them I ask for alternatives. When weighing time commitments and code impacts, I find this "expose local constructor" refactoring to be of real benefit and frequently used.