Search code examples
c#unit-testinginterface

Should all public methods be declared in the interface?


Given an interface like:

public interface IFoo
{
    string Bar();
}

And a class that implements it like:

public class Foo : IFoo
{
    public string Bar()
    {
        returns "Bar";
    }

    public string SomeOtherMethod()
    {
        returns "Something else";
    }
}

Is there a problem with this code? Should we add all methods to the interface?

One example: imagine a private method that grows complex enough to need unit tests. Could you make it public (so it can be called from the test project) but not add it to the interface (because no clients need to call it)?


Solution

  • If what a class does is simple enough that we can test its public methods and private methods at the same time, then we can just test the public methods.

    If the private method grows so complex that between the public and private method we need too many combinations of tests then it's time to separate the private method into its own class. Making the private method public would break the encapsulation of the class. Even if we don't add the method to an interface, making the class method public still adds the method to the public interface of the class itself.

    So if we have this:

    public class ClassWithComplexPrivateMethod
    {
    
        public void DoSomething(int value)
        {
            PrivateMethodThatNeedsItsOwnTests(value);
        }
    
        private string PrivateMethodThatNeedsItsOwnTests(int value)
        {
            // This method has gotten really complicated!
            return value.ToString();
        }
    }
    

    We might refactor to something like this:

    public interface IRepresentsWhatThePrivateMethodDid
    {
        string MethodThatNeedsItsOwnTests(int value);
    } 
    
    public class RefactoredClass
    {
        private readonly IRepresentsWhatThePrivateMethodDid _dependency;
    
        public RefactoredClass(IRepresentsWhatThePrivateMethodDid dependency)
        {
            _dependency = dependency;
        }
    
        public string DoSomething(int value)
        {
            return _dependency.MethodThatNeedsItsOwnTests(value);
        }
    }
    

    And now a new class implements IRepresentsWhatThePrivateMethodDid.

    Now when we test the refactored class we mock IRepresentsWhatThePrivateMethodDid, and we write separate unit tests for any classes that implement IRepresentsWhatThePrivateMethodDid.

    It may seem like a contradiction to say that exposing the private method as public breaks encapsulation, but exposing it as its own separate class doesn't. There are two differences:

    • The refactored class doesn't depend on the new class containing what used to be the private method. It depends on the new interface.
    • The interaction between the refactored class and the interface is still hidden within its methods. Other classes that call its public methods don't "know" about how it uses its dependency. (In fact, other classes will likely depend on abstractions instead of directly on the refactored class.)

    It's also easy to get carried away with this and introduce the separate dependency too soon, when we could have tested the class - including its private methods - through the public methods. I've done this lots of times, and it can result in lots and lots of unnecessary interfaces and extra classes. There's no perfection, just our best efforts to balance it.


    One more thought: We tend to use interfaces to represent dependencies, but we don't have to. If what we're extracting was just a single private method, then perhaps we can represent it with a delegate or a Func instead, like this:

    public class RefactoredClass
    {
        private readonly Func<int, string> _dependency;
    
        public RefactoredClass(Func<int, string> dependency)
        {
            _dependency = dependency;
        }
    
        public string DoSomething(int value)
        {
            return _dependency(value);
        }
    }
    

    Or we can use a delegate, which I like better than Func because it indicates what the function does.