Real-World Refactoring: .Sort() vs .OrderBy() in C#

Do you use LINQ's OrderBy for quicker sorting or do you old-school it with .Sort()? Today, we'll discuss this and the answer may surprise you as to which one is better.

Over the weekend, I was updating my TableTreeView which uses a combination of C#, JavaScript, and SignalR for the dynamic loading of TreeViewItems.

The way the TableTreeView works is you pass in a List<T> and the TableTreeView wraps code around the list.

Think of each TableTreeItem as a header record (master) wrapping the single entity (detail) where it's similar to a master/detail relationship.

public class TableTreeItem<T> where T: class
{
    public int Id { get;set; }
    public int Order { get;set; }
    public T Item { get; set; }
    public int? ParentId { get;set; }
    public int Level { get;set; }
    public bool HasChildren { get; set; }
    public bool IsRoot { get; set; }
    public bool Expanded { get; set; }
}

While I was examining and refactoring the C# code, I noticed the sorting routine was hard-coded. It was sorting the list by ID using the LINQ method OrderBy().

Yuck. Ughh. Pfffffftt.

Why do you think this bothered me?

What if I wanted to sort by something different? Maybe title or even item type (folder or item)? Maybe both?

What's Wrong With The Code?

The current code base uses a fluent API (similar to LINQ), but there isn't anything available for the sorting.

This was the line of code causing me to have a tick.

var records = _results.OrderBy(e => e.Id);

If it's only going to be used once, there isn't a problem.

If I'm using this TableTreeView for multiple projects (which I am), you want this to be easy for the developer (and that includes me!).

Based on the SOLID principles, this looks like a job for "O" (Open/Closed principle). This means Open for extension, Closed for modification. If we wanted to keep this code, it would violate the Open/Closed Principle because we would have to go into the class and change the LINQ's OrderBy extension.

It would be easier to pass in some kind of sort routine.

Now, since this is a tool I'm using heavily in numerous places throughout a personal project, this type of sorting with the OrderBy wasn't going to work for me. It HAS to be sortable on a number of levels.

So, how can we sort out this mess (pun intended) and make it more extensible?

Time To Refactor!

The first thing was to make it easier to pass in a sort routine.

One way is to use LINQ. We can pass in a predicate like using a Repository pattern with a Where clause similar to:

public IEnumerable<Project> GetByCustomer(Expression<Func<Project, bool>> projectFilter)
{
    return Find(projectFilter);
}

We could pass in a sort predicate.

var treeView = new MvcTableTree<ProductCategory>(items)
    .SortBy(treeViewItem => treeViewItem.Item.Title)
    .Build();

Our MvcTableTree code would have this method.

private Func<TableTreeItem<T>, string> _sortRoutine;

public MvcTableTree<T> SortBy(Func<TableTreeItem<T>, string> sortRoutine)
{
    _sortRoutine = sortRoutine;
    return this;
}

When the time comes to "Build()" our TreeView, our sort inside our Build method would be as easy as:

var records = _results.OrderBy(_sortRoutine);

Now we have an open way to sort our TreeViewItems.

Compound Sorting

Once I put this code in place, I continued to work on the project.

Not even an hour later, I found a place in the project where I needed a deeper way to sort the TreeViewItems.

I had to sort the Item by two keys, HasChildren (descending) and Title. The reason to sort by HasChildren as descending was to display the folders first and then sort the titles.

The single predicate for the LINQ OrderBy would not work.

Of course, we could always do a workaround by passing in a concatenated string of 0 for an item and greater than 0 for a folder and concatenate the title on the end like this:

var treeView = new MvcTableTree<ProductCategory>(items)
    .SortBy(treeViewItem => (treeViewItem.HasChildren ? 1.ToString() : 0.ToString())+treeViewItem.Item.Title)
    .Build();

but this just feels too hacky.

We need a better way.

You Comparer? No, IComparer

One of the overloads of the .Sort() method includes an IComparer interface.

This IComparer interface has one method signature called Compare.

The best part about this interface is you can write as many sorting objects as you need by creating a Bridge pattern and passing them in to perform any type of sorting imaginable.

First, we need to modify our Builder pattern to accept a different kind of Sort parameter.

var treeView = new MvcTableTree<ProductCategory>(items)
    .SortBy(new SortByHierarchy<ProductCategory>())
    .Build();

Our new .SortBy() method takes a different parameter in our MvcTableTree code.

private IComparer<TableTreeItem<T>> _sortFunc;

public MvcTableTree<T> SortBy(IComparer<TableTreeItem<T>> sortRoutine)
{
    _sortFunc = sortRoutine;
    return this;
}

All we need to do now is create our Sort class called SortByHierarchy().

public class SortByHierarchy<T> : IComparer<TableTreeItem<T>> where T: class
{
    public int Compare(TableTreeItem<T> x, TableTreeItem<T> y)
    {
        var cat1 = x.Item as ProductCategory;
        var cat2 = y.Item as ProductCategory;

        return cat1.ProductCategoryId.CompareTo(cat2.ProductCategoryId);
    }
}

If you need to pass in a different sorting class, your Compare method would be different when building your MvcTableTree.

Conclusion

There are many ways to write sorting code, but I feel this technique embodies the "O" in SOLID.

Don't get me wrong, the .OrderBy() clause in LINQ has it's place, yet it doesn't feel open enough.

With this solution, you are free to create any type of sort algorithm using a class and being able to pass it into the code without digging into the MvcTableTree class to modify it.

Open for Extension, Closed for Modification.

Was this an easy refactoring? Is there a better way to use LINQ's .OrderBy()? Could you extend .ThenBy()? Thoughts? Post your comments below and let's discuss.