How can I improve this sorting code? [closed]

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
– Cody Gray ♦, Commented May 11, 2012 at 19:49
I would recomend using LINQ composition. See stackoverflow.com/questions/5881107/… — Euphoric
– Euphoric, Commented May 11, 2012 at 19:50
Create a dictionary of delegates and then call them basing on the key(s). — alex
– alex, Commented May 11, 2012 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
– Rod, Commented May 11, 2012 at 19:51

BrokenGlass · Accepted Answer · 2012-05-11 19:50:04Z

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, 2012 at 19:50

BrokenGlass

161k28 gold badges297 silver badges339 bronze badges

Sign up to request clarification or add additional context in comments.

11 Comments

Mathew Thompson Over a year ago

That's still a lot of boilerplate for all the fields, wouldn't reflection be much cleaner? Imagine if he had 50 fields! :)

Euphoric Over a year ago

@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.

BrokenGlass Over a year ago

@mattytommo: That's certainly possible and neat if this is an in-memory collection

Niklas B. Over a year ago

@Euphoric: Oh, that certainly would be a problem. I don't know C# too well :)

Elian Ebbing Over a year ago

@Euphoric - You could create a Dictionary<string,IComparable>.

|

Mathew Thompson · Accepted Answer · 2012-05-11 19:50:35Z

2

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();

answered May 11, 2012 at 19:50

Mathew Thompson

56.5k15 gold badges131 silver badges151 bronze badges

Comments

Community · Accepted Answer · 2017-05-23 10:28:22Z

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, 2017 at 10:28

CommunityBot

11 silver badge

answered May 11, 2012 at 19:52

Tejs

41.4k10 gold badges71 silver badges89 bronze badges

Collectives™ on Stack Overflow

How can I improve this sorting code? [closed]

3 Answers 3

11 Comments

Comments

Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

11 Comments

Comments

Comments

Linked

Related