CODEWITHSUNDEEP
c#unit-testingarchitecturerefactoring
Bob Tway
Bob Tway

Reputation: 9613

How to design testable code bound up with non-testable functions

Imagine a class like this:

public class FileParser : IFileParser
{
    public string ParseFirstRowForDelimiters(string path)
    {
        using (TextFieldParser parser = new TextFieldParser(path))
        {
            string line = parser.ReadLine();

            if(lineContains("'"))
            {
                return "'";
            }

            if(lineContains("\"")
            {
                return "\"";
            }

            return "";
        }
    }
}

For classes which depend on the FileParser, I can mock its functions via its Interface and all is fine. However, there's logic inside the class itself which is dependent on the TextFieldParser returning a line to check.

I cannot "interface out" the TextFieldParser with a mock in order to unit test that logic since it's an external class from Microsoft that has no interface.

I could push off the if statements into separate functions like this:

public bool HasSingleQuote(string lineToCheck)
{
    return lineToCheck.Contains("'");
}

But these don't need to be accessible outside the class. Nor do they really need to be called from elsewhere so they don't belong in a helper class or suchlike. So, by good design principles, they out to be private not public, and I should test them via their public accessor. Which, in this case, depends on the untestable TextFieldParser.

I could wrap the TextFieldParser in my own class and stick and interface on that, but it feels like overkill and unnecessary replication of code.

I appreciate this is a trivial example that isn't really worth testing, but I just threw it together to illustrate the issue. What's the best approach to refactoring this code to make my logic testable?

Upvotes: 2

Views: 287

Answers (2)

Nkosi
Nkosi

Reputation: 247521

I would say test what you own. TextFieldParser is an implementation detail. MS would have extensively tested it's functionality for release. if the concern is about the logic inside of its implementation where you are doing your conditional check then it could be argued that the IFileParser implementation may be doing too many things. I'm reminded of SRP and having only one reason to change.

public interface IDelimiterLogic {
    string Invoke(string line);
}

with an implementation like

public class DefaultDelimiterLogic : IDelimiterLogic {
    public string Invoke(string line) {
        if (line.Contains("'")) {
            return "'";
        }

        if (line.Contains("\"")) {
            return "\"";
        }

        return "";
    }
}

The FileParser implementation would then be refactored to...

public class FileParser : IFileParser {
    IDelimiterLogic delimiterLogic;
    public FileParser(IDelimiterLogic delimiterLogic) {
        this.delimiterLogic = delimiterLogic;
    }

    public string ParseFirstRowForDelimiters(string path) {
        using (TextFieldParser parser = new TextFieldParser(path)) {
            string line = parser.ReadLine();
            return delimiterLogic.Invoke(line);
        }
    }
}

So now if you want to test your delimiter logic the system under test would be the IDelimiterLogic implementation.

UPDATE:

credit to @JAllen as well for abstracting 3rd party dependencies.

public interface ITextFieldParser : IDisposable {
    bool EndOfData { get; }
    string ReadLine();    
}

public interface ITextFieldParserFactory {
    ITextFieldParser Create(string path);
}

public class TextFieldParserFactory : ITextFieldParserFactory {
    public ITextFieldParser Create(string path) {
        return new TextFieldParserWrapper(path);
    }
}

public class TextFieldParserWrapper : ITextFieldParser {
    TextFieldParser parser;
    internal TextFieldParserWrapper(string path) {
        parser = new TextFieldParser(path);
    }
    public bool EndOfData { get{ return parser.EndOfData; } }
    public string ReadLine() { return parser.ReadLine(); }
    public void Dispose() { parser.Dispose(); }
}

New refactored IFileParser implementation

public class FileParser : IFileParser {
    IDelimiterLogic delimiterLogic;
    ITextFieldParserFactory parserFactory;

    public FileParser(IDelimiterLogic delimiterLogic, ITextFieldParserFactory parserFactory) {
        this.delimiterLogic = delimiterLogic;
        this.parserFactory = parserFactory;
    }

    public string ParseFirstRowForDelimiters(string path) {
        using (ITextFieldParser parser = parserFactory.Create(path)) {
            string line = parser.ReadLine();
            return delimiterLogic.Invoke(line);
        }
    }
}

Upvotes: 2

J. Allen
J. Allen

Reputation: 592

The testing issue is based on the fact that TextFieldParser is a 3rd party dependency, correct? One strategy you could use is to wrap that 3rd party dependency in a service interface that you then pass in to your FileParser.

public interface ITextFieldParserService
{
   string ReadLine();
}

public class DefaultTextFieldParserService : ITextFieldParserService
{
   private TextFieldParser parser;
   public ITextFieldParserService Setup(string path)
   {
       parser = new TextFieldParser(path);
   }
   //you'd want some teardown method to dispose of TextFieldParser, or make
   //the service IDisposable probably
}

public class FileParser : IFileParser
{
   public FileParser(ITextFieldParserService textParserService)
   {
   }
   ...
   public string ParseFirstRowForDelimiters(string path)
   {
       var parser = textParserService.Setup(path)        
        string line = parser.ReadLine();

        if(lineContains("'"))
        {
            return "'";
        }

        if(lineContains("\"")
        {
            return "\"";
        }

        return "";         
   }

You can have a default implementation of that service that actually uses the 3rd party TextFieldParser, but you can also write a test implementation that just returns a set of pre-defined data.

Upvotes: 2

Related Questions