Be Careful with ToList()

I recently came across some code during a review that seemed perfectly fine at first glance, but upon further inspection, had potential to perform terribly. Take a look. What’s wrong with this code?

using (var context = new DbContext())
{
context.SomeTable.Where(t => t.Id == someId)
.GroupBy(t => t.Category)
.Select(tg => new { tg.Category, Profit = tg.Sum(p => p.Profit) })
.ToList()
.ForEach(SaveToDb);
}

Do you see it? It’s easy to miss.

Calling ToList() on the Enumerable materializes the query, iterating over the Enumerable in order to generate the List. Then, just a moment later, we iterate over the List. We’ve potentially doubled the run time of this method. In most cases, likely not a big deal, but what if the List contains thousands of items? We’re doing more work than necessary here.

Fixing this is a simple matter of storing the IEnumerable in a variable and using the traditional foreach syntax instead of the ForEach extension method.

using (var context = new DbContext())
{
var query = context.SomeTable.Where(t => t.Id == someId)
.GroupBy(t => t.Category)
.Select(tg => new { tg.Category, Profit = tg.Sum(p => p.Profit) });

foreach(var item in query)
{
SaveToDb(item);
}
}

It just goes to show that you really have to be paying attention when reviewing (or writing!) code. Some issues can be terribly difficult to spot at a glance.

Replacing simple factories with Func<TResult> delegate

I’ve recently noticed the emergence of a recurring pattern in my code. Each project has the same class in it, and this duplication has really been bothering me. The solution to this repetition just hit me this morning while I was reading Jon Skeet’s “C# In Depth.”

First, let me illustrate the pattern I was using and why it’s a problem.

I’ve been refactoring 1-tiered legacy code into something testable. This has involved creating a data layer class, typically called DataProvider which implements an IDataProvider interface so that I can easily mock out the data access.  Now, because any kind of data access is extremely likely to have unmanaged resources, IDataProvider extends IDisposable.

Mocking the VBA Editor: Moq an IEnumerable that Works with Linq

It’s been a rough morning here. I’ve just spent six hours trying to properly create a mock for the VBProject interface. There’s very little information out there about this, so I thought I’d take a moment to jot this down and save someone else the headache. For all the grief this gave me, the solution is amazing simple.

Using Hampers Testing: Enter the Factory

I’ve been writing a lot of C# lately, and paying a lot of attention to my test coverage while I’m at it. Everything was going great until I wanted to use the FolderDialogBrowser to let my users select a directory.

FolderDialogBrowser implements IDisposable, so I naturally reached for a Using block.

using (var folderPicker = new FolderBrowserDialog)
{
    if (folderPicker.ShowDialog() != DialogResult.OK)
    {
        return;
    }

    //...
}

Then I stopped dead. I can’t do that. This will display a GUI and any hope of running automated tests against this method is lost.