6

Having issues with the OrderBy clause not having any impact on the sort. I have walked through this in the debugger and insuring this is a case that the sort line of the code is being hit and reviewing the results after it the order by has not been applied.

public static IEnumerable<DDLOptions<TValueType>> GetDDLOptionsViewModel<TClass, TValueType>(
            IEnumerable<TClass> list, 
            Func<TClass, TValueType> value, 
            Func<TClass, string> displayText,
            bool sort = true
        )
        {
            List<DDLOptions<TValueType>> ddlOptions;

            ddlOptions = list.Select(
                l => new DDLOptions<TValueType>
                        {
                            Value = value(l),
                            DisplayText = displayText(l)
                        }
                    ).ToList();  <========== Works if I put the Order By here.

            if (sort)
            {
                ddlOptions.OrderBy(l => l.DisplayText); <===== Does NOT work here.
            }

            return ddlOptions;
        }

5 Answers 5

13

OrderBy returns a query that would perform the ordering: it does not modify the original list (whereas something like List<T>.Sort would modify the original)

Instead try something like:

ddlOptions = ddlOptions.OrderBy(l => l.DisplayText).ToList();

You might want to play around with the type of ddlOptions or where/how you return the data as we're doing an extra ToList than probably necessary, but that's probably a minor/non-issue for this case anyway.

Note that the same applies to other LINQ functions like GroupBy, Distinct, Concat - all return results rather than modifying the source collection.

5
  • 1
    The easiest would be to just move .ToList() to the return statement. Jan 18, 2013 at 16:47
  • @MattiasBuelens Probably, the only thing is as ddlOptions is defined, you wouldn't be able to reassign it: OrderBy returns an IOrderedEnumerable<T> not a List<T>. That's why I'm saying Chad might have to tweak the types or return logic. Jan 18, 2013 at 16:49
  • True, but that should be an easy fix. var could be useful here as well. Jan 18, 2013 at 16:50
  • @MattiasBuelens It would as long as Chad doesn't use the initial ToList or add any additional Linq that doesn't return IEnumerable<DDLOptions<TValueType>> explicitly. Personally, I'd probably favour just returning right in the if block and avoid type/IEnumerable reassignment in this case. But like you said, all this is pretty trivial/easy and up to Chad. Jan 18, 2013 at 16:55
  • Yes, I delay the ToList to be called by the calling methods. Thank you all for the help! Everyone provided similar answers, but this one was technically correct because I was initially defining ddlOptions as a list (List<DDLOptions<TValueType>> ddlOptions). In the end I just made it an IEnumerable and did not call ToList as it is called in the calling method.
    – crichavin
    Jan 18, 2013 at 18:40
6

Try:

if (sort)
{
    ddlOptions = ddlOptions.OrderBy(l => l.DisplayText); <===== Should work now.
}
3
  • It would be better if the .ToList() call was moved to the return statement so that the unordered list is not calculated. Jan 18, 2013 at 16:46
  • @Venson Chad would have to redefine ddlOptions to something like IEnumerable<DDLOptions<TValueType>> for this assignment to work. Jan 18, 2013 at 16:51
  • Don't seems so, If i am not Completely Wrong, this dos not matter because T is still the same just this List Interface is changed, and the Data should not take care of it.
    – Venson
    Jan 18, 2013 at 16:59
3

As others have said, you need to assign the result of OrderBy to something as it doesn't mutate the sequence it acts on. It's easiest to make ddlOptions an IEnumerable instead of a List, so that you can assign the result to that. The ToList call on the select is also not needed:

public static IEnumerable<DDLOptions<TValueType>> GetDDLOptionsViewModel<TClass, TValueType>(
        IEnumerable<TClass> list,
        Func<TClass, TValueType> value,
        Func<TClass, string> displayText,
        bool sort = true
    )
{
    IEnumerable<DDLOptions<TValueType>> ddlOptions;

    ddlOptions = list.Select(
        l => new DDLOptions<TValueType>
                {
                    Value = value(l),
                    DisplayText = displayText(l)
                }
            );

    if (sort)
    {
        ddlOptions = ddlOptions.OrderBy(l => l.DisplayText);
    }

    return ddlOptions;
}

Note that this version of the method will use deferred execution, and so won't actually perform the Select/OrderBy until the sequence is iterated. If you don't want to do that, you can add ToList on the return line.

1

You need to type:

ddlOptions = ddlOptions.OrderBy(l => l.DisplayText);
1
  • I'm an idiot...of course...I forgot to assign it to itself. Thanks Soner! I did not have to call the ToList on it for it to work.
    – crichavin
    Jan 18, 2013 at 16:51
1

OrderBy doesn't sort a List<T> or any other IEnumerable<T>. It produces a new, sorted IEnumerable<T>. So calling ddlOptions.OrderBy(...) doesn't modify ddlOptions.

If you have a List<T> and wish to sort it, you can use the Sort method - in particular the overload that takes a Comparison<T> as a parameter. This actually sorts the list instead of returning a new IEnumerable.

Comparison<T> is a delegate representing a function that takes two of T and returns a negative number if the first is "less" than the second, a positive number if the first is "greater" than the second, and zero if one isn't sorted before or after the other.

In this case you don't have to remember that. Instead, you can just do this:

ddlOptions.Sort((x, y) => string.CompareOrdinal(x.DisplayText, y.DisplayText));

You're passing in a function that takes two items in the list and returns the comparison result of their DisplayText properties, which will be negative, 0, or positive.

Sometimes we use OrderBy because it doesn't modify the original list. But if modifying the list is what we want then we can use Sort.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.