HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Unit testing legacy code with static classes

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
staticwithtestingunitclassescodelegacy

Problem

I am adding new functionality into a legacy application. For my new classes I have introduced Unit Testing. I still need to work with the existing code.

One of the problems I face is that a lot of static methods have been used.

Below is an example of a class I need to use:

public class HelperClass
{
    public static bool ReturnSomething()
    {
        return true;
    }
}


My fix for making this testable:

public interface IHelperClass
{
    bool ReturnSomething();
}

public class HelperClass : IHelperClass
{
    public static bool ReturnSomething()
    {
        return true;
    }

    bool IHelperClass.ReturnSomething()
    {
        return ReturnSomething();
    }
}


A class that used to use the static method, but by introducing a property (initialized by the constructor), that now uses the testable class. I can inject a mock using this property.
Note that I could have used a factory for injecting the mock, however I wanted to keep it simple for the example.

public class UserClass
{
    public UserClass()
    {
        // Initialize with default 
        HelperClass = new HelperClass();
    }

    public IHelperClass HelperClass { get; set; }
}


My advantages:

  • No need to modify existing code that is using the static class



  • Code can be migrated from static to a testable instance



  • I can reuse the existing concrete class



My disadvantages:

  • Needed to modify legacy code that needs to be tested. Not always possible when in external assembly.



  • PropertyName is the same as the class Name



I welcome any feedback!

Solution

I happen to like your fix for making it testable, but would reverse the implementation order between the static method and the explicit interface method. This will allow you to slowly change existing code first to use the Singleton instead of the static and then finally use an injectable pattern:

public interface IHelperClass
{
    bool ReturnSomething();
}

public sealed class HelperClass : IHelperClass
{
    private static readonly IHelperClass instance = new HelperClass();

    private HelperClass()
    {
    }

    public static IHelperClass Instance
    {
        get
        {
            return instance;
        }
    }

    [Obsolete("Use the Instance singleton's implementation instead of this static method.")]
    public static bool ReturnSomething()
    {
        return Instance.ReturnSomething(); // Simply calls the Singleton's instance of the method.
    }

    bool IHelperClass.ReturnSomething()
    {
        return true; // The original static ReturnSomething logic now goes here.
    }
}

Code Snippets

public interface IHelperClass
{
    bool ReturnSomething();
}

public sealed class HelperClass : IHelperClass
{
    private static readonly IHelperClass instance = new HelperClass();

    private HelperClass()
    {
    }

    public static IHelperClass Instance
    {
        get
        {
            return instance;
        }
    }

    [Obsolete("Use the Instance singleton's implementation instead of this static method.")]
    public static bool ReturnSomething()
    {
        return Instance.ReturnSomething(); // Simply calls the Singleton's instance of the method.
    }

    bool IHelperClass.ReturnSomething()
    {
        return true; // The original static ReturnSomething logic now goes here.
    }
}

Context

StackExchange Code Review Q#10359, answer score: 12

Revisions (0)

No revisions yet.