What is the best practice for refactoring a static method in order to make it more testable?

Question

Let's say you have a static method that looks something like this:

public static bool Foo()
{ 
    var bar = new Bar();
    //do some stuff here
}

This method as it stands can be a real headache to unit test.

What is the best practice to refactor this so that it can be testable, without turning it into an instance method or changing the method signature?

Answer 1

It really depends on what Bar is. If it's something simple then your Foo method is already testable, you just need to specify your expectations and invoke it statically, e.g.:

Assert.IsTrue( FooContainer.Foo() );

But if Bar encapsulates, say, your database access layer, then you can't test Foo without a real database, which is why (thanks @ysolik), static methods are death to testability. Or, in the words of Michael Feathers, "don't hide a TUF in a TUC" (TUF stands for a test-unfriendly feature, TUC stands for a test-unfriendly construct). If Bar is indeed test-unfriendly, then sorry, it doesn't work well without making Foo an instance method. You would need to redesign your code first:

public class FooContainer {
    public bool Foo() {
        var bar = new Bar();
        //...
    }
}

When Foo is no longer static, you can invoke it on an instance of FooContainer:

var container = new FooContainer();
Assert.IsTrue( container.Foo() );

The next step is to extract an interface from Bar (let's call it IBar) and inject it into FooContainer:

public class FooContainer {
    private readonly IBar m_bar;
    public FooContainer( IBar bar ) { m_bar = bar; }
    public bool Foo() {
        // don't create another Bar, use m_bar
    }
}

Now you can mock/stub IBar with your favourite isolation framework and test your FooContainer code in isolation from its dependencies.

Answer 2

What is the point of Foo? What does it do? What is a Bar?

It seems from your question that Bar is a class that introduces side effects, or Bar is a resource.

Either way, in both circumstances, without changing the method signature, you're hooped without going into the ill-fated world of pre-processor directives (#if test var bar = FakeBar(); // = bad).

If Bar is a class that introduces side effects: without injecting that dependency or returning whatever it affects, you're in trouble.

If it's a resource (Stream, DBConnection, etc.) then the only realistic options I can see is to:

1. Extract out an interface and take an IBar as a parameter like @azheglov says

2. Create a fake/stub that inherits from Bar and pass that as a parameter

What it boils down to is that you're likely going to have to change the method signature, reduce side-effects and not create hidden dependencies if you want to make it easier to test.

Answer 3

Actually, you have quite a lot of options. I ran into a similar problem today and figured that it all depends on how hard it is to instantiate an object of type Bar.

Bar is easily instantiable

If Bars constructor doesn't have any Parameters or all of them are available somehow you may do the following:

1. Add a parameter of type Bar to Foos signature (we will fix the signature change later):

   public static bool Foo(Bar bar)
   {
       // ...
       var baz = bar.Baz();
       bar.Quxx(42);
       // more stuff
   }
   

2. Create an overloaded version of Foo without the Bar parameter and let it create the instance and pass it along:

   public static bool Foo()
   {
       return Foo(new Bar());
   }
   

3. Extract an interface IBar and use it in the signature of the new Foo method. Use the compiler errors to find out which methods and properties you need to pull up into the interface:

   public interface IBar
   {
       object Baz();
   
       void Quxx(int q);
   }
   
   public class Bar : IBar
   {
       // Implementations of Baz, Quxx, and other stuff
   }
   
   public static class SomeStaticClass
   {
       public static bool Foo()
       {
           return Foo(new Bar());
       }
   
       public static bool Foo(IBar bar)
       {
           // ...
           var baz = bar.Baz();
           bar.Quxx(42);
           // more stuff
       }
   }
   

4. Use your favourite mocking technique/framework to test the Foo method. It is still a static method and you don't need to change its callers.

It is really hard to instantiate a Bar object

But what if Bars constructor has some weird parameters. Maybe we need to pass values we compute inside of Foo. I found the following approach to be quite elegant.

1. We need some way to invoke Bars constructor in the production code, but want to supply some mock in testing code, so we have to introduce some indirection. Let's use the factory pattern and create a BarFactory that creates the Bar instance for us. We then pass the factory instance to the static method:

   public class Bar
   {
       public Bar(Baz baz, Quxx quxx)
       {
           // do stuff with baz and quxx, launch a nuclear bomb, etc.
       }
   }
   
   public class BarFactory
   {
       public Bar CreateBar(Baz baz, Quxx quxx)
       {
           return new Bar(baz, quxx);
       }
   }
   
   public static class SomeStaticClass
   {
       public static bool Foo(BarFactory factory)
       {
           // compute baz and quxx
           var bar = factory.CreateBar(baz, quxx);
           // use bar
       }
   
       public static bool Foo()
       {
           return Foo(new BarFactory());
       }
   }
   

2. As in item 3 in the above case we can now extract the IBar interface and let the compiler help us to pull up only the members we need.

3. Now we can extract an interface for the factory, too. The final code will look like this:

   public interface IBar
   {
       // IBars methods
   }
   
   public class Bar : IBar
   {
       public Bar(Baz baz, Quxx quxx)
       {
           // do stuff with baz and quxx, launch a nuclear bomb, etc.
       }
   }
   
   public interface IBarFactory
   {
       IBar CreateBar(Baz baz, Quxx quxx);
   }
   
   public class BarFactory : IBarFactory
   {
       public IBar CreateBar(Baz baz, Quxx quxx)
       {
           return new Bar(baz, quxx);
       }
   }
   
   public static class SomeStaticClass
   {
       public static bool Foo(IBarFactory factory)
       {
           // compute baz and quxx
           var bar = factory.CreateBar(baz, quxx);
           // use bar
       }
   
       public static bool Foo()
       {
           return Foo(new BarFactory());
       }
   }
   

4. Create mocks for IBarFactory and IBar and use them in your tests. You will have preserved Foos signature and it is still static.