LINQ follow-up: a little more work was needed

20 August 2008

Oops! The code example I had in my LINQ posting yesterday was inaccurate and didn't actually do anything! That code was from a changeset I was preparing on my machine and it had to go through the gauntlet that is an ironclad code review by Ted Glaza. He made a good catch here, and it's a mistake I'm not likely to make again.

I suppose the mistake was easy enough to make: the sequence that OrderBy returns provides the sorted data, since LINQ's extension methods don't alter the source. My method was a no-op since the "classes" list was never updated.

The incorrect code I had to perform a sorting:

static void SortTestClasses(IList<ITestClass> classes)
{
    classes.OrderBy(a => a.Name);
}

That line pretty much did nothing but some extra work, leaving the input list "classes" untouched. Here's what I should have been doing:

List<ITestClass> sorted = classes.OrderBy(a => a.Name).ToList();
classes.Clear();
foreach (var item in sorted)
{
    classes.Add(item);
}

Luckily, I am using extension methods in my project already, and have one that performs an operation similar to System.Collections.Generic.List::AddRange, but permits this operation on generic list interfaces as well. Here's that extension method:

/// <summary>
/// An AddRange implementation for the generic IList interface.
/// </summary>
/// <typeparam name="TListType">The list type.</typeparam>
/// <param name="list">The list object.</param>
/// <param name="collection">The collection to copy into the list.</param>
public static void AddRange<TListType>(this IList<TListType> list, IEnumerable<TListType> collection)
{
    if (list == null || collection == null)
    {
        return;
    }

    foreach (TListType value in collection)
    {
        list.Add(value);
    }
}

With that, the code sample is then a little simpler:

List<ITestClass> sorted = classes.OrderBy(a => a.Name).ToList();
classes.Clear();
classes.AddRange(sorted);

Now to make the code into a single useful line, I created another extension method called "Replace". The method creates a temporary list with the sequence's list (that will become the sorted data), clears the original list, and then adds the collection into the list. Thank goodness for your average mom and pop having 8GB of memory... I'm going for the clean-looking, BCL-intense solution, as opposed to pretending that I'm doing assembly programming on my TI-86...

/// <summary>
/// Replace a list's contents with the items in the IEnumerable.
/// </summary>
/// <typeparam name="TListType">The list type.</typeparam>
/// <param name="list">The list object.</param>
/// <param name="sequence">The sequence to copy into the list.</param>
public static void Replace<TListType>(this IList<TListType> list, IEnumerable<TListType> sequence)
{
    if (list == null)
    {
        return;
    }
    if (sequence == null)
    {
        throw new ArgumentNullException("sequence");
    }

    List<TListType> copy = sequence.ToList();
    list.Clear();
    list.AddRange(copy);
}

And then the updated OrderBy statement. Here's what my final version should have been yesterday:

/// <summary>
/// Sorts the test classes alphabetically by name.
/// </summary>
/// <param name="classes">A list of test class metadata objects.</param>
public static void SortTestClasses(IList<ITestClass> classes)
{
    classes.Replace(classes.OrderBy(a => a.Name));
}

Well, sorry if I mislead anyone yesterday. And thanks Ted!

Jeff Wilcox is a Software Engineer at Microsoft in the Open Source Programs Office (OSPO), helping Microsoft engineers use, contribute to and release open source at scale.

comments powered by Disqus