Search code examples
c#linq

How do I specify LINQ's OrderBy direction as a boolean?


I have a simple data class that has this signature:

internal interface IMyClass {
    string Letter { get; }
    int Number { get; }
}

I wish to be able to sort this data based on a field (specified as string sortField) and a direction (specified as bool isAscending)

Currently I am using a switch (with the ascending logic within each case as if)

IEnumerable<IMyClass> lst = new IMyClass[];//provided as paramater
switch (sortField)
{
    case "letter":
        if( isAscending ) {
            lst = lst.OrderBy( s => s.Letter );
        } else {
            lst = lst.OrderByDescending( s => s.Letter );
        }
        break;
    case "number":
        if( isAscending ) {
            lst = lst.OrderBy( s => s.Number );
        } else {
            lst = lst.OrderByDescending( s => s.Number );
        }
        break;
}

This is pretty ugly, for 2 properties, but when the sort logic differs it becomes a problem (we also see s => s.Number is duplicated twice in the code)

Question What is the best way to pass a boolean to choose the sort direction?

What I've Tried I've ripped apart System.Core.dll and found the OrderBy Extension method implementations:

OrderBy:

public static IOrderedEnumerable<TSource> OrderBy<TSource, TKey>(
        this IEnumerable<TSource> source, 
        Func<TSource, TKey> keySelector
    ){

    return new OrderedEnumerable<TSource, TKey>(
        source, 
        keySelector, 
        null, 
        false
    );
}

OrderByDescending:

public static IOrderedEnumerable<TSource> OrderByDescending<TSource, TKey>(
        this IEnumerable<TSource> source, 
        Func<TSource, TKey> keySelector
    ){
        return new OrderedEnumerable<TSource, TKey>(
            source, 
            keySelector, 
            null, 
            true
        );
}

It appears the purpose of having 2 named methods is to abstract this boolean away. I can't create my own extension easily as OrderedEnumberable is internal to System.Core, and writing a layer to go from bool -> methodName -> bool seems wrong to me.


Solution

  • I'd say write your own extension method:

    public static IEnumerable<T> Order<T, TKey>(this IEnumerable<T> source, Func<T, TKey> selector, bool ascending)
    {
        if (ascending)
        {
            return source.OrderBy(selector);
        }
        else
        {
            return source.OrderByDescending(selector);
        }
    }
    

    Then you can just write:

    lst = lst.Order( s => s.Letter, isAscending );
    

    As for specifying method name: I hope this doesn't come off as a cop-out answer, but I think you should stick with using a selector function instead of passing in a string. Going the string route doesn't really save you any typing or improve clarity (is "letter" really much faster or clearer than s => s.Letter?) and only makes your code fatter (you'd either need to maintain some sort of mapping from strings to selector functions or write custom parsing logic to convert between them) and possibly more fragile (if you go the latter route, there's a pretty high probability of bugs).

    If your intention is to take a string from user input to customize sorting, of course, you have no choice and so feel free to disregard my discouraging remarks!


    Edit: Since you are accepting user input, here's what I mean by mapping:

    class CustomSorter
    {
        static Dictionary<string, Func<IMyClass, object>> Selectors;
    
        static CustomSorter()
        {
            Selectors = new Dictionary<string, Func<IMyClass, object>>
            {
                { "letter", new Func<IMyClass, object>(x => x.Letter) },
                { "number", new Func<IMyClass, object>(x => x.Number) }
            };
        }
    
        public void Sort(IEnumerable<IMyClass> list, string sortField, bool isAscending)
        {
            Func<IMyClass, object> selector;
            if (!Selectors.TryGetValue(sortField, out selector))
            {
                throw new ArgumentException(string.Format("'{0}' is not a valid sort field.", sortField));
            }
    
            // Using extension method defined above.
            return list.Order(selector, isAscending);
        }
    }
    

    The above is clearly not as clever as dynamically generating expressions from strings and invoking them; and that could be considered a strength or a weakness depending on your preference and the team and culture you're a part of. In this particular case, I think I'd vote for the hand-rolled mapping as the dynamic expression route feels over-engineered.