Writing tests alongside your code can often be helpful. It can help you think of edge cases where things might go wrong, provide a safety net when refactoring, and help you optimise your architecture for low coupling and reuse. Adding tests is relatively easy when you have enough time and are working on a greenfield project (i.e. you don’t have to worry about existing code). However, we don’t always have this luxury. The code we work with may have been written under different circumstances and/or without testing in mind.

In this article we’ll look at an example of some (possibly legacy) code you might typically come across. And we’ll also explore a few ways that we might go about to add tests to it.

A Private Affair

Let’s imagine we’re working on an existing codebase. It contains an (admittedly unimaginatively named) service that’s responsible for doing some important and complex calculations. There’s one entry point (RunCalculation); calling it will run two sub-calculations. Each sub-calculation will compute its own result and then write it to a service-wide variable (_mainResult). Finally, RunCalculation returns the value of _mainResult.

As a sidenote, this approach isn’t thread-safe. One variable is shared across the service instance; usage from multiple threads could result in one thread’s sub-calculations overwriting another’s, potentially making the overall result unreliable.

public class ComplexCalculationService
{
    private double _mainResult;

    public double RunCalculation(double input)
    {
        _mainResult = input;
        SubCalculation1();
        SubCalculation2();
        return _mainResult;
    }

    private void SubCalculation1()
    {
        var subResult1 = _mainResult;
        // More calculations to transform subResult1...

        _mainResult = subResult1;

    }

    private void SubCalculation2()
    {
        var subResult2 = _mainResult;
        // More calculations to transform subResult2...

        _mainResult = subResult2;
    }
}

The service is an important part of the overall product, and we want to know as soon as possible if its results become inaccurate. The most obvious and readily available action is to write tests comparing the output of RunCalculation against values known to be correct. This is indeed useful. But it’s difficult to know where the miscalculation occurs if they fail, especially if the two transformations are long and complex. It would be helpful if we could narrow things down - if we could test each sub-calculation.

A More Functional Approach

It’s easier to write tests for pure functions. Let’s first make some minor changes to our service so it no longer relies on state variables. Instead, the sub-calculations will use variables we pass to them and return their results. These can then be chained together to calculate the final result.

In addition to simplifying our tests, these changes also make our service thread-safe. As it’s no longer using shared variables, a single instance of ComplexCalculationService could potentially be used across multiple threads safely (and without synchronisation locks) if required.

public class ComplexCalculationService
{
    public double RunCalculation(double input)
    {
        var result1 = SubCalculation1(input);
        var finalResult = SubCalculation2(result1);
        return finalResult;
    }

    private double SubCalculation1(double input)
    {
        var subResult1 = input;
        // More calculations to transform subResult1...

        return subResult1;
    }

    private double SubCalculation2(double input)
    {
        var subResult2 = input;
        // More calculations to transform subResult2...

        return subResult2;
    }
}

Increasing Code Visibility

We’re almost ready to write tests for the sub-calculations. However, we still can’t access them externally as they’re private – we need to change their visibility modifiers to be more permissive. But what do we change them to?

Going Public

The most obvious option is public. But let’s pause for a moment to think about what this means: anything will be able to access the methods. Anything. While this is good from a testing perspective, it also invites other code modules to access the sub-calculations too. This isn’t necessarily a bad thing, especially if there’s a valid case for code reuse. But that wasn’t the original design/intention. And there’s a possibility that with this reuse, our (entire) service becomes a dependency for another module, including parts that may be unrelated. This could lead to two unintended consequences:

• Creating unnecessary coupling between modules.

• Making it easier to create circular dependencies.

Instead of directly changing the visibilities from private to public, a good strategy would be to refactor the sub-calculations into public methods on a separate service. This separation of concerns lessens the risk of these consequences. It also follows the Single Responsibility Principle (the S of SOLID) as each module would have its own dedicated purpose.

Staying Protected

The next option is protected. While this doesn’t directly make our sub-calculations accessible, it lets us create a subclass of ComplexCalculationService with ways to expose the inherited methods we want for testing. This can work and might be advantageous if we have other services in our solution that should also inherit the protected members. However, in such cases we’re arguably better off refactoring the sub-calculations into a separate service as previously mentioned; doing so would also make testing easier as we wouldn’t have to maintain a separate version of the service just for testing.

Being Internally Restricted

The last option we’ll consider is internal. Members marked as internal are usually only accessible to other classes within the same project. However, we can use the InternalsVisibleTo attribute to make them also accessible in any external modules we choose. Applying it is simple, but the process differs slightly depending on your project format.

Non-SDK-style projects (the default for .NET Framework) will have an AssemblyInfo.cs file in the Properties node of the project. We can add the following line to it, replacing Project.Name with the name of the assembly containing our tests:

[assembly: InternalsVisibleTo("Project.Name")]

For SDK-style projects, we need to add the following directly to the .csproj file. Again, we should replace Project.Name with an appropriate value.

<ItemGroup>
    <InternalsVisibleTo Include="Project.Name" />
</ItemGroup>

Making a Decision

With three choices available, which is the best one?

As with many things in programming, the answer is it depends. I’ve worked on a few codebases and have found myself usually opting for either:

• Refactoring the code into a separate service, or

• Making members Internal.

Creating a new service is usually the cleanest option. It’s also a chance to review a module’s dependencies, reducing them to only what’s necessary. Another advantage is that as a separate module, it can be substituted or mocked in tests where appropriate.

However, if time is a limiting factor (or if refactoring isn’t cost-effective) I’ll mark the methods to be tested as internal. While more visible than originally intended, they’re still hidden to external modules (except those explicitly named). The compromise of being visible in a limited way and thoroughly tested feels safer than the alternative: being correctly hidden but potentially faulty.

Summary

Tests can be helpful, but we sometimes need to work with codebases that were created without. In these cases, one option is to add them before making significant changes to act as a safety net. Writing tests for existing/legacy code can be difficult if it wasn’t written with testing in mind. However, there are a few tips to make things a bit simpler.

Where possible, it will be easier if you can convert methods into functions that aren’t reliant on variables on the state. You then need to make them accessible for testing. Where appropriate, the best choice might be to refactor the methods into their own service (or services). If this isn’t possible (or suitable), you might want to change some private methods to have greater visibility.

public, protected, and internal are all valid options. internal members are usually only accessible to other members of the same project, but you can cheat – the InternalsVisibleTo attribute will let you make them visible to your test projects too.

Thanks for reading!

This article is from my newsletter. If you found it useful, please consider subscribing. You’ll get more articles like this delivered straight to your inbox (once per week), plus bonus developer tips too!