# File text searcher

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;

-

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));

-
Saying something is slow over 100,000 iterations is not worth mentioning. This is code review and without the entire application being profiled, what is it slow compared to? Avoid premature optimization and optimize based on slowdowns across the application. You're talking about nano second differences here. I'm going to take a stab in the dark and say that you did these tests in Debug mode as well yes? –  Vince Panuccio Jun 12 at 4:22
@VincePanuccio They were. I was just curious to see if there was any kind of a difference - I'm not saying query syntax is slow, I'm just saying the syntax I prefer seems to be faster. FWIW I edited that part out. –  Mat's Mug Jun 12 at 4:25
Thanks, a lot of good stuff here. One note: I'm pulling the extensions list out of config, and I don't want to rely on someone else remembering to put the dot in. So I'm keeping it as a replace. –  adamcodes Jun 12 at 17:04

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.

 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

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)

-

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

-
True, but the file list will be filtered by 1+ extensions (I wanted to keep the question simple, so I used a static list). I figured since the extensions list will be variable I would grab all and then filter. There's probably a way to pass in the desired extensions, so I'll check that out next. Thanks. –  adamcodes Jun 12 at 17:00

var variables are still strongly-typed. It is dynamic variables that are not strongly typed. –  ANeves Jun 19 at 10:48