C# – How to refactor these functions to foster code-reuse

cdelegatesoptimization

I wrote these helper functions a few weeks ago and I feel like I'm not taking advantage of some C# language features that would keep me from writing these same loops again for other similar helpers.

Can anyone suggest what I'm missing?

public static IList<string> GetListOfLinesThatContainTextFromList(
        Stream aTextStream, IList<string> aListOfStringsToFind)
{
    IList<string> aList = new List<string>();

    using (var aReader = new StreamReader(aTextStream))
    {
        while (!aReader.EndOfStream)
        {
            var currLine = aReader.ReadLine();

            foreach (var aToken in aListOfStringsToFind)
                if (currLine.Contains(aToken))
                    aList.Add(currLine);
        }
    }

    return aList;
}

public static DataTable GetDataTableFromDelimitedTextFile(
        Stream aTextStream, string aDelimiter)
{
    var aTable = new DataTable();
    Regex aRegEx = new Regex(aDelimiter);

    using (var aReader = new StreamReader(aTextStream))
    {
        while (!aReader.EndOfStream)
        {
            // -snip-
            // build a DataTable based on the textstream infos
        }
    }

    return aTable;
}

Best Answer

I'd use composition here. For example, for the first one, first split out the bit reading lines - and make it lazy...

public static IEnumerable<string> ReadLines(Stream input)
{
    // Note: don't close the StreamReader - we don't own the stream!
    StreamReader reader = new StreamReader(input);
    string line;
    while ((line = reader.ReadLine()) != null)
    {
        yield return line;
    }
}

(You can find a more fully-featured version of this in MiscUtil.)

Now you can use LINQ to filter out lines not in an appropriate set (and I would use a HashSet rather than a list, unless it's a really short list):

var acceptedLines = new HashSet<string>();
// Populate acceptedLines here
var query = ReadLines(input).Where(line => acceptedLines.Contains(line))
                            .ToList();

You can use the same line-reading method when populating the DataTable. You need to be careful though - this is lazily evaluated so you need to keep the stream open until you've finished reading, and then close it yourself. I prefer to pass in something which lets you get at a Stream (or a TextReader) as then the closing logic can be in the ReadLines method.

Related Topic