Static methods and testability

Question

I have an application service which returns entity by its registration number

public Entity FindByRegNumber(string number)
{
    if (!RegNumber.IsValid(number))
    {
        return null;
    }

    var regNumber = new RegNumber(regNumber);
    return repository.FindBy(regNumber);
}

Here RegNumber is a value object and it has static method IsValid which verifies that string correctly represents registration number. This method is used inside constructor as well, but, if string is not valid, constructor throws InvalidOperationException.

If IsValid method belonged to some injected dependency, I could just check in my test that IsValid was called (a kind of white box testing). Now I have to test FindByRegNumber repeating all the input used for testing IsValid method (black box testing).

It looks like the only reason to get rid of static method is to make testing easier. It does not even decreases coupling while call to FullRegistrationNumber constructor stays.

Another approach is to introduce a factory FullRegistrationNumberCreator' which could abstract away both validation and construction ofFullRegistrationNumber`, but it seems like an overkill and overengineering.

Is here a correct solution?

Answer 1

First thing to point out is that I disagree with the way you test validity and create instance of RegNumber.

I would design it in a way that there is only one way to create RegNumber instance and that also also includes validity check.

For example something like

public static bool RegNumber.TryParse(string number, out RegNumber regNumber)
{
  //...
}

So to create new instance of RegNumber, you have to call this method. This then introduces structural constraint, that if something uses RegNumber type, it is using valid one.

Then, I would take the pragmatic approach and say, that the code you present doesn't have any interesting logic to test. And it is enough to write two high-level integration tests. First to check with valid registration number, second with invalid registration number. No need to test whole registration invalidation again.

Answer 2

I'd extract the concept of a RegNumberValidator into a class of its own.

Its instance could have a virtual bool IsValid(string) method, such that you can overwrite it for testing - and also for derived classes requiring additional validation steps.

If you prefer a static class/method, you can still use a trick described by M. Feathers in his "Working with Legacy Code", e.g.

public static class RegNumberValidator
{
    private IRegNumberValidator _instance = new NonstaticRegNumberValidator();

    public void SetInstanceForTesting(IRegNumberValidator testInstance)
    {
        _instance = testInstance;
    }

    public bool IsValid(string regNumber)
    {
        return(_instance.IsValid(regNumber));
    }
}

Note: some people think that this is a hack which should be applied in the cases of legacy code only - because there you can't get ahead without hacks. In my opinion, it is a valid pattern.