Note: Jon Skeet sent me a few pointers on simplifying my Linq usage a little - my original code went a little overboard on the query operators. I've amended things appropriately below - thanks Jon!
Back in July, I was doing a code review with a team member for a new feature. One of the methods we were reviewing looked like this (Some names have been changed to protect the codebase)
private string GetTag(string originalTag, Gadget gadget)
{
if (gadget.Details.Modules != null)
{
foreach (Widget widget in gadget.Details.Widgets)
{
if (widget != null && widget.Options != null)
{
foreach (Option option in widget.Options)
{
if (option != null && option.Skus != null)
{
foreach (Sku sku in option.Skus)
{
if (sku != null && sku.Id == IdToMatch)
{
return originalTag + gadget.Id;
}
}
}
}
}
}
}
return originalTag;
}
My thought at the time was, "That's a lot of looping for that one test on sku.Id." My second thought was, “I know I‘ve seen that foreach nesting a dozen times in our code." I started thinking about how else we could express the intent of something like the above code. (Seriously – the app does this kind of iterating a lot). What I really wanted was a way to capture and re-use the looping logic, and separate that out from the action being taken during iteration. My first thought was to just write a method that did all the looping and took an Action delegate as an argument, as the action to be done through the innermost loop. But that seemed clumsy, and I wasn’t satisfied with it.
I hadn’t put much thought into it until I was reading chapters 5 and 6 in Jon Skeet’s C# in Depth last night, and reviewed a little about C# 2.0-style iterators. I started thinking that maybe the way to break this out was to introduce some custom iterators. And then I remembered the C# 3.0 linq expressions are basically tersely-written iterator produces, and I knew I was on to something. Here's what I did:
Start Small
The first thing I notice when looking at the above loop is that there’s a pattern to the collection access:
1) Check if the collection itself is null and don’t iterate if it is.
2) When looping through the collection, skip over null entries.
Let's start with the first one:
public static class EnumerableExtensions
{
public static IEnumerable OrEmpty(this IEnumerable collection)
{
return collection ?? Enumerable.Empty();
}
}
This is an extension method for all IEnumerable instances. If the instance is non-null, it just returns it. But if the instance is null, it returns Enumerable.Empty, which represents an empty collection of type T. Usage of this extension method would look like this:
IEnumerable<int> collection = null;
foreach(var c in collection.OrEmpty())
{
//Code goes here. Doesn’t run when collection is empty.
}
Note that we can call the extension method on collection even when it’s null – that’s because it’s really being passed in as an argument to a static method, so there’s no NullReferenceException possible here.
Now that we can safely handle a null collection, how can we skip past the null entries that could be lurking inside a collection? Let’s add another extension method:
public static IEnumerable SkipNulls(this IEnumerable collection) where T : class
{
return collection.Where(c => c != null);
}
The Where method returns an IEnumerable that will bypass all the null entries in the collection. That means we can write code like this now:
var strings = new[] { "a", "b", null, "c", null, "d", null, null, "e" };
foreach (var s in strings.SkipNulls())
{
Console.Write(s.ToUpper());
}
This produces the string “ABCDE” to the console. Note that we don’t have to test the variable “s” for nullity before calling ToUpper on it, as the SkipNulls method is ensuring that we don’t iterate over the null entries in the collection.
In our original code, those two checks were almost always grouped together. So let’s create a third extension method:
public static IEnumerable NullSafe(this IEnumerable collection) where T : class
{
return collection.OrEmpty().SkipNulls();
}
Which just chains the results of the two methods together.
Apply It
With these three methods in place, our original code (remember our original code?) looks a little different:
public static string GetTag (Gadget gadget, string originalTag)
{
foreach (var widget in gadget.Details.Widgets.NullSafe())
{
foreach (var option in widget.Options.NullSafe())
{
foreach (var sku in option.Skus.NullSafe())
{
if (sku.Id == IdToMatch)
{
return originalTag + gadget.Id;
}
}
}
}
return originalTag;
}
Now we can kind of start to see the intent of the method come out. This is good, but it can get better. Let’s turn that search for a matching sku id into a linq expression:
public static string GetTag (Gadget gadget, string originalTag)
{
var hasSpecialSku = (from m in gadget.Details.Widgets.NullSafe()
from o in m.Options.NullSafe()
from s in o.Skus.NullSafe()
select s).Any(s => s.Id == IdToMatch);
return hasSpecialSku ? originalTag + gadget.Id : originalTag;
}
Wow. That’s a much shorter and more succinct method than we started with! This LINQ expression is selecting all the skus from all the options from all the widgets. The Any method returns true if it finds a sku that matches the predicate we provide (in this case, a test for whether the Id matches a given constant.)
Going All In
Ok, this is already pretty good. But here’s the thing – this method isn’t the only place where I want to iterate over widgets, or down into options, or skus. There are a lot of places where I want to do this kind of thing. Let’s add some more extension methods, only this time, let’s add some extensions to the Gadget class:
public static IEnumerable<Widget> AllWidgets(this Gadget gadget)
{
return gadget.Details == null
? Enumerable.Empty<Widget>()
: gadget.Details.Widgets.NullSafe();
}
public static IEnumerable<Option> AllOptions(this Gadget gadget)
{
return gadget.AllWidgets().SelectMany(w => Options.NullSafe());
}
public static IEnumerable<Sku> AllSkus(this Gadget gadget)
{
return gadget.AllOptions().SelectMany(o => o.Skus.NullSafe());
}
These build on each other as we work our way down the nesting. Note that AllWidgets has an extra test to make sure the Details property isn’t null. We can’t use our NullSafe extension here, as Details doesn’t return a collection. So, we look at the field, and if it’s null, we return Empty.
Now what does this do to our original method?
public static string GetTag(Gadget gadget, string originalTag)
{
var exists = gadget.AllSkus().Any(s => s.Id == IdToMatch);
return exists ? originalTag + gadget.Id : originalTag;
}
I’m betting that this isn’t the only place where we’ll need to know if a sku Id matches, and I’m betting this isn’t the only place we want to know if a Gadget contains that sku. You guessed it, more extension methods:
public static bool IsSpecial(this Sku s)
{
return s.Id == IdToMatch;
}
private static bool HasSpecialSku(this Gadget gadget)
{
return gadget.AllSkus().Any(IsSpecial);
}
Which makes our original method:
public static string GetTag(Gadget gadget, string originalTag)
{
return gadget.HasSpecialSku()
? originalTag + gadget.Id
: originalTag;
}
Dividends on our Investment
Wow. We got that original beast, with 7 nested levels of conditional code, down to one statement. In the process, we introduced a whole new set of powerful abstractions that we can use throughout our entire code base to do similar kinds of simplifications, and a base we can build on as we identify and capture more of those similar abstractions. Even with just what we've done so far, we can now do things like:
//All Discounted gadget.AllWidgets().Where(m => m.WidgetType == WidgetType.Discount);
//Number of options priced above 50 dollars. gadget.AllOptions().Where(o => o.OriginalPrice > 50.00).Count();
//The first sku of the first option of the first widget. gadget.AllSkus().First();
The code we factored out definitely ends up being longer than the original nested for-loops, but each method does one thing, is reusable across the system, and can be easily understood and tested in isolation. And, as you can see from the three examples above, we've made that cost back and then some as soon as we re-use any of the new code.
Wrap it Up
Through a series of small incremental changes applying the powerful new language features of C# 3.0, we were able to transform our code from something large, error-prone, and repetitive into something small, easily understood, and unique. We also unlocked a whole new set of productivity-enhancing and intention-revealing capabilities in our system. And I spent longer typing up this post than I did actually coding it!