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

Wrapping a CSV file for access and testing

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

Problem

I have a simple class that represents and accesses the data from a CSV file (using the CsvHelper library). I've tried restructuring the code to allow for better unit testing, but I'm sure the structure of the class could be better; both for testing and for using it (I'm not sure if there is any standard recommendations for these types of classes).

Here is the class code:

public class CsvCountFile
{
    public string AbsolutePath { get; }
    public string Delimiter { get; }

    public LocationDefinition LocationData { get; private set; }
    public List CountData { get; private set; }

    public CsvCountFile(string absolutePath, string delimiter = ",")
    {
        AbsolutePath = absolutePath;
        Delimiter = delimiter;
    }

    public void ReadCountData()
    {
        using (var fileReader = File.OpenText(AbsolutePath))
        {
            ReadCountData(fileReader);
        }
    }

    public void ReadCountData(TextReader fileReader)
    {
        CountData = new List();

        using (var csvReader = new CsvReader(fileReader))
        {
            csvReader.Configuration.HasHeaderRecord = false;
            csvReader.Configuration.RegisterClassMap();
            csvReader.Configuration.RegisterClassMap();

            csvReader.Read(); // get header
            csvReader.Read(); // get first record
            LocationData = csvReader.GetRecord();

            csvReader.Read(); // skip blank line
            csvReader.Read(); // skip second header section

            while (csvReader.Read())
            {
                var count = csvReader.GetRecord();
                CountData.Add(count);
            }
        }

    }
}


The ReadCountData was split into two so that the reading logic could be tested by forming a TextReader in memory, instead of needing an actual csv file. While this code works, I don't foresee any usage scenarios that would typically require reading the csv by passing in a TextReader.

Also, due to this structur

Solution

Just passing by. There are few comments I want to make here.

-
ReadCountData() is definitely out of place because the first method just exposes you to a StreamReader object reference. This method can not be unit-tested. I created an interface called IFile that wraps the static OpenText() method

public  interface IFile
{
    StreamReader OpenText(string path);
}



I created a FileClass that uses this interface

public  class FileClass
{
    private readonly IFile file;

    public FileClass(IFile file)
    {
        this.file = file;
    }

    // I renamed your ReadCountData as OpenFile
    public void OpenFile(string path)
    {
        try
        {
            var fileReader = file.OpenText(path);              
        }
        catch (Exception e) {
            // do whatever;
        }            
    }
}


-
Using this approach, you can unit-test the OpenText() using the below code

[TestClass]
public class UnitTest1
{
    [TestMethod]
    public void OpenFile_PathAsAParameter_CSVFileOpened()
    {

        //Arrange
        string path = @"C:\myfilerocks.txt";
        var fileMock = new Moq.Mock();
        var fileClass = new FileClass(fileMock.Object);

        //Act
        fileClass.OpenFile(path);

        //Assert
        fileMock.Verify(x=> x.OpenText(path));
    }
}


Notice there is still a dependency on StreamReader and TextReader, it's best to create a wrapper for those classes. When you have some many static class in a testable method I see it more as a smell . When a wrapper can't be created for them keep them as dependency e.g public void ReadCountData(TextReader fileReader) seems alright to me. If you are looking for wrappers for the file system, visit System.IO.Abstractions by Jonathan Channon. His blog also explains a great deal Abstracting the File System

So the above test that the OpenText() was called.

-
Rather than using csvReader, I will use TextFieldParser from Microsoft.VisualBasic.FileIO.TextFieldParser . It works for both C# and VB. Note this example was extracted from Reading CSV Files using C#

using (TextFieldParser parser = new TextFieldParser(@"c:\temp\test.csv"))
{
    parser.TextFieldType = FieldType.Delimited;
    parser.SetDelimiters(",");
    while (!parser.EndOfData) 
    {
        //Processing row
        string[] fields = parser.ReadFields();
        foreach (string field in fields) 
        {
           //TODO: Process field
        }
    }
}


Note: AbsolutePath, Delimiter is redundant here.
Refrain from names like ReadCountData, they don't provide information about their functionality. I would've thought ReadDataCount sounds better.

Code Snippets

public  interface IFile
{
    StreamReader OpenText(string path);
}
public  class FileClass
{
    private readonly IFile file;

    public FileClass(IFile file)
    {
        this.file = file;
    }

    // I renamed your ReadCountData as OpenFile
    public void OpenFile(string path)
    {
        try
        {
            var fileReader = file.OpenText(path);              
        }
        catch (Exception e) {
            // do whatever;
        }            
    }
}
[TestClass]
public class UnitTest1
{
    [TestMethod]
    public void OpenFile_PathAsAParameter_CSVFileOpened()
    {

        //Arrange
        string path = @"C:\myfilerocks.txt";
        var fileMock = new Moq.Mock<IFile>();
        var fileClass = new FileClass(fileMock.Object);

        //Act
        fileClass.OpenFile(path);

        //Assert
        fileMock.Verify(x=> x.OpenText(path));
    }
}
using (TextFieldParser parser = new TextFieldParser(@"c:\temp\test.csv"))
{
    parser.TextFieldType = FieldType.Delimited;
    parser.SetDelimiters(",");
    while (!parser.EndOfData) 
    {
        //Processing row
        string[] fields = parser.ReadFields();
        foreach (string field in fields) 
        {
           //TODO: Process field
        }
    }
}

Context

StackExchange Code Review Q#135892, answer score: 2

Revisions (0)

No revisions yet.