How can I improve this sorting code?

Question

I'm checking the sort parameter and building a bunch of if statements:

if (sortDirection == "ASC")
{
    if (sortBy == "Id")
        return customerList.OrderBy(x => x.Id).Skip(startIndex).Take(pageSize).ToList();
    if (sortBy == "FirstName")
        return customerList.OrderBy(x => x.FirstName).Skip(startIndex).Take(pageSize).ToList();
    if (sortBy == "City")
        return customerList.OrderBy(x => x.City).Skip(startIndex).Take(pageSize).ToList();
}
else
{
    if (sortBy == "Id")
        return customerList.OrderByDescending(x => x.Id).Skip(startIndex).Take(pageSize).ToList();
    if (sortBy == "FirstName")
        return customerList.OrderByDescending(x => x.FirstName).Skip(startIndex).Take(pageSize).ToList();
    if (sortBy == "City")
        return customerList.OrderByDescending(x => x.City).Skip(startIndex).Take(pageSize).ToList();
}

How do I make this better?

In what way do you want to "improve" it? Does it not work as intended? Is it too slow? Do you not like how the code is structured? We need more information here. — Cody Gray - on strike, May 11 '12 at 19:49
I would recomend using LINQ composition. See http://stackoverflow.com/questions/5881107/how-can-i-build-entity-framework-queries-dynamically/5882243#5882243 — Euphoric, May 11 '12 at 19:50
Create a dictionary of delegates and then call them basing on the key(s). — alex, May 11 '12 at 19:51
It'd be nice if it can be more dynamic where I don't need to add if statements just because a new sortBy string has been sent in. — Rod, May 11 '12 at 19:51
Not too familiar with C# syntax, but one way I would try to do it would be to use a hashmap to store the string(ex: Id) to objects(ex x.Id) associations and just retrieve your orderBy keys from that hashmap. — Philippe, May 11 '12 at 19:53
See this similar question: http://stackoverflow.com/questions/1493274/linq-sort-direction-from-string — hatchet - done with SOverflow, May 11 '12 at 19:54

score 8 · Accepted Answer · answered May 11 '12 at 19:50

8

Separate your ordering and the rest of the query - the parts that are the same for each query you don't have to duplicate in your codebase (keep it DRY):

var query = customerList;

if (sortDirection == "ASC")
{
    if (sortBy == "Id")
       query = query.OrderBy(x => x.Id);
    ///and so on
}

query = query.Skip(startIndex).Take(pageSize).ToList();

answered May 11 '12 at 19:50

BrokenGlass

158,293
28
286
335

That's still a lot of boilerplate for all the fields, wouldn't reflection be much cleaner? Imagine if he had 50 fields! :) – Mathew Thompson May 11 '12 at 19:56
@NiklasB. That would be problem, because OrderBy is generic and the lambda has different type for every property. Unless you want to convert it to object everywhere. – Euphoric May 11 '12 at 19:57
@mattytommo: That's certainly possible and neat if this is an in-memory collection – BrokenGlass May 11 '12 at 19:59
@Euphoric: Oh, that certainly would be a problem. I don't know C# too well :) – Niklas B. May 11 '12 at 20:01
@Euphoric - You could create a `Dictionary`. – Elian Ebbing May 11 '12 at 20:11
@ElianEbbing But then, you cannot use LINQ, but need to apply this to the list itself. – Euphoric May 11 '12 at 20:13
@Euphoric - Ehm, I meant `Dictionary>`. I typed too fast ;) – Elian Ebbing May 11 '12 at 20:15
@Elian: Yeah, something like that is what I had in mind too. Functions should be contravariant in the argument types and covariant in the return value, which seems to be the case in C# as well. – Niklas B. May 11 '12 at 20:17
@ElianEbbing I still dont know what are you going to do with IComparable in LINQ. Check OrderBy method signatures. There is no IComparable everywhere. It only needs function to return value, that is used for comparison. And even IComparare is generic with type of this property. .. maybe implementing the sorting by itself and return object itself from function? That might work. – Euphoric May 11 '12 at 20:18
@Euphoric - Yes, I just noticed that there is no `IComparable` in the signature (which is strange, how do you sort on a type that is not `IComparable`?). However, `OrderBy()` accepts both `Func` and `Func`. You don't have to write anything yourself for that. – Elian Ebbing May 11 '12 at 20:26
@Euphoric - I created an example: http://pastebin.com/ymuDxCM8 – Elian Ebbing May 11 '12 at 20:34

score 2 · Answer 2 · answered May 11 '12 at 19:50

Use reflection :)

customerList = (sortDirection == "ASC")
   ? customerList
        .OrderBy(x => x.GetType().GetProperty(sortBy).GetValue(x, null))
        .Skip(startIndex)
        .Take(pageSize)
        .ToList()
   : customerList
        .OrderByDescending(x => x.GetType().GetProperty(sortBy).GetValue(x, null))
        .Skip(startIndex)
        .Take(pageSize)
        .ToList();

score 1 · Answer 3 · edited May 23 '17 at 10:28

1

It looks like you simply want to order by the property name as a string. In which case, this is actually already solved by using "Dynamic LINQ":

Dynamic LINQ OrderBy on IEnumerable<T>

Take a look at this question's answer and it should provide you with sample code to solve your problem.

edited May 23 '17 at 10:28

Community

1
1

answered May 11 '12 at 19:52

Tejs

40,736
10
68
86

How can I improve this sorting code?

3 Answers3

Linked