patterncsharpMinor
Wrapping a CSV file for access and testing
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:
The
Also, due to this structur
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.
-
I created a
-
Using this approach, you can unit-test the OpenText() using the below code
Notice there is still a dependency on
So the above test that the OpenText() was called.
-
Rather than using
Note:
Refrain from names like
-
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() methodpublic interface IFile
{
StreamReader OpenText(string path);
}I created a
FileClass that uses this interfacepublic 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 SystemSo 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.