Multi-tiered sorting using custom IComparer

Question

I have this custom IComparer I use to sort a list of SurveyResponse objects, but it seems really inefficient, both in terms of runtime performance, and code elegance and maintainability. I thought of implementing IComparable for each of these components, and each CompareTo knowing its "parent" to compare, but the sort order for this report I'm making won't necessarily be the same as other sort orders, so that may not work.

It looks like the sort of problem that should be solvable using recursion, but maybe I'm just over-analyzing it?

Are there any best practices that would make this faster and more maintainable?

private class CommentComparer : IComparer<SurveyResponse>
{
  public int Compare(SurveyResponse x, SurveyResponse y)
  {
    // Sort by Application Name...
    int result = x.Question.Survey.Feature.Application.Name.CompareTo(
      y.Question.Survey.Feature.Application.Name);
    if (result != 0) { return result; }
    // ...then Feature Name...
    result = x.Question.Survey.Feature.Name.CompareTo(
      y.Question.Survey.Feature.Name);
    if (result != 0) { return result; }
    // ...then SurveyTime...
    result = x.Question.Survey.TimeTaken.CompareTo(y.Question.Survey.TimeTaken);
    if (result != 0) { return result; }
    // ...then ID...
    result = x.Question.Survey.ID.CompareTo(y.Question.Survey.ID);
    if (result != 0) { return result; }
    // ...then Position.
    return x.Question.Position.CompareTo(y.Question.Position);
  }
}

Answer 1

You can use types from ComparerExtensions to make your code more maintainable:

IComparer<SurveyResponse> comparer = KeyComparer<SurveyResponse>
    .OrderBy(sr => sr.Question.Survey.Feature.Application.Name)
    .ThenBy(sr => sr.Question.Survey.Feature.Name)
    .ThenBy(sr => sr.Question.Survey.TimeTaken)
    .ThenBy(sr => sr.Question.Survey.ID)
    .ThenBy(sr => sr.Question.Position);

But I don't think you can actually make your comparer more efficient, it should be already very fast (it's just a few simple instructions). Are you sure your performance problem is caused by this comparer?

Answer 2

+1 to @svick of course. Here is another solution without NList, LINQ, etc., maybe somebody find it interesting. Creating a separate IComparer for every comparison also could work, for example:

private class ApplicationNameComparer : IComparer<SurveyResponse> {
    public int Compare(SurveyResponse x, SurveyResponse y) {
        return x.Question.Survey.Feature.Application.Name.
            CompareTo(y.Question.Survey.Feature.Application.Name);
    }
}

then put them into a list:

var comparers = new List<IComparer<SurveyResponse>>();
comparers.Add(new ApplicationNameComparer());
comparers.Add(new FeatureNameComparer());
comparers.Add(new SurveyTimeComparer());
comparers.Add(new IdComparer());
comparers.Add(new PositionComparer());

finally iterate through the list:

foreach (var comparer in comparers) {
    int result = comparer.Compare(x, y);        
    if (result != 0) {
        return result;
    }
}
return 0;

It removes the logic repetition of if (result != 0) { return result; } but it uses more classes. On the other hand every class has single responsibility and they do something more complex together.

References:

• http://en.wikipedia.org/wiki/Single_responsibility_principle
• http://c2.com/cgi/wiki?FearOfAddingClasses

Answer 3

As the creator of NList, I can confirm that @palacsint's solution is how NList is implemented internally.

Another way to implement it is using the chain-of-command pattern.

public class CompoundComparer<T> : IComparer<T>
{
    private readonly IComparer<T> first;
    private readonly IComparer<T> second;

    public CompoundComparer(IComparer<T> first, IComparer<T> second)
    {
        this.first = first;
        this.second = second;
    }

    public int Compare(T x, Ty)
    {
        int result = first(x, y);
        if (result != 0)
        {
            return result;
        }
        return second(x, y);
    }
}

Once you've created this class, you can compose them together to build more complex comparisons.

IComparer<T> comparer = new CompoundComparer<T>(
    comparison1,
    new CompoundComparison(
        comparison2,
        comparison3));

Some helper functions can make this easier to read/write. At that point, though, it is probably just easier to steal code from NList. I wrote it so you don't have to.