File text searcher

Question

I have a working version of a logfile searcher that targets a specific directory by extension type, and searches the files for certain words.

I'm wondering if my method is the most efficient implementation, or if there are things I could do better.

string startFolder = @"C:\logs\"; 

// search within these extensions
List<string> searchExtensions = new List<string>("log", "csv");

DirectoryInfo dir = new DirectoryInfo(startFolder);

// get files 
IEnumerable<FileInfo> fileList = dir.GetFiles("*.*", SearchOption.AllDirectories)
               .Where(file => searchExtensions.Contains(file.Extension.Replace(".", "")));

// strings to search for 
List<string> lSearchTerms = new List<string>() { "info", "error", "warning" };

// GetFileText simply returns the contents of the file
var queryMatchingFiles = 
                from file in fileList
                let fileText = GetFileText(file.FullName)
                where lSearchTerms.Any(val => fileText.Contains(val))
                select file.FullName;

Answer 1

List<string> searchExtensions = new List<string>("log", "csv");

This doesn't need to be a List<T> - you're really only using it because it's an IEnumerable<T>, so this would suffice:

var searchExtensions = new[] { "log", "csv" };

And when you're using it...

IEnumerable<FileInfo> fileList = dir.GetFiles("*.*", SearchOption.AllDirectories)
               .Where(file => searchExtensions.Contains(file.Extension.Replace(".", "")));

You're calling .Replace(".", "") to match your searchExtensions items... that replacement would be unnecessary if you had the dot in your extensions in the first place:

var searchExtensions = new[] { ".log", ".csv" };
var files = dir.GetFiles("*.*", SearchOptions.AllDirectories)
               .Where(file => searchExtensions.Contains(file.Extension));

Side note, it's often preferable to use string.Empty over "", to better communicate intent.

The name fileList is lying: it's an IEnumerable<T>, not a List<T>. Hence, files is a better, simpler name.

List<string> lSearchTerms = new List<string>() { "info", "error", "warning" };

I'm not sure why you've prefixed the searchTerms with a lowercase "L", but again, you're declaring a List<T> when all you need is an IEnumerable<T> - again, a simple array would do:

var searchTerms = new[] { "info", "warning", "error" };

Listing the log levels you're interested in, in ascending order, makes the terms seem less arbitrarily chosen. If you're using NLog for logging, Error isn't the highest log level available; are you missing "fatal"?

Also IEnumerable<T>.Contains() is case-sensitive, which means if you're searching for info and your files contain Info or INFO, you're not getting all the results you indend to retrieve. More on this here

I'm not sure why the last instruction is using var instead of explicitly declaring an IQueryable<string>, but you should probably wrap the query in parentheses and call .ToList() and declare it as an IEnumerable<string> instead.

I might have a bias against the LINQ query syntax, but I have ran some tests, and this was slightly faster than the query syntax:

var searchResults = files.Where(file => terms.Any(term => File.ReadAllText(file.FullName).Contains(term)))
                         .Select(file => file.FullName);

---

Side note about comments: there's too many of them. They are redundant and often only rephrase what the code is already saying; comments should say why, not what.

// filter doesn't support regex, so get all files *and then* filter by extension:
var files = dir.GetFiles("*.*", SearchOptions.AllDirectories)
               .Where(file => searchExtensions.Contains(file.Extension));

Answer 2

First of all I'll suggest to fix (or make explicit) the behavior if the extension is uppercase: important.CSV will not be considered by this software. At least a comment in the code.

Now about the performance:

 where lSearchTerms.Any(val => fileText.Contains(val))

in this way for every file that doesn't match you do 3 full scan of the file content, 2+1/2 if contains only "warning", 1+1/2 if contains only "error" and 1/2 if contains "info". Also if "Contains" implements the best possible string search algorithm (not sure), it's not the best algorithm for this use (search using a finite set of patterns)

Some algorithms that solve optimally this problem are

1. Rabin–Karp
2. Commentz-Walter
3. Rabin–Karp

More info here

If you want to avoid this solution I suggest to use a standard regex: at least it will make the search only one time (and a fixed pattern search can be modeled quite easily with an automa)

Answer 3

There is one thing you could have done more efficient. When you read the files you wrote [star.star] which means that all files, regardless of type will get returned. This might be insufficient if there are many different files that are not related to your program in the target directory

Answer 4

@AdamCodes - Based on all your responses so far above, here are my comments and suggestions:

1. Remember to remove only the first dot from the extension. We have multi-dotted extensions (like ".aspx.cs"). Your above code will make these files "aspxcs" which will not match anything.

2. When you load the extensions array/list/collection, make two copies of every extension configured. One uppercase, one lower. Always remove the "." if prefixed with one.

3. Contains() is case sensitive and also slow. Instead, use RegEx to perform the search. Yes, your code will grow a little more, but will be more efficient.

RE: "var", I am not a supporter of using "var" and prefer strong-typing my variables.