5

In my C# code, I need to evaluate two non-null variables. I worked out a set of if-else if statements, but in my mind it looks ugly and a little bit too sloppy, even if it is correct.

I looked in the MSDN Library and saw only examples for selection based on a single variable.

Is there a cleaner and more compact way to achieve the same outcome?

Update: I filled in code to provide more context. Looking at this more, perhaps I can manipulate the linq query directly based on the parameters. However, the question I pose is the generic one that I would like to focus on: the selection rather than the code used after the selection is made.

public ActionResult Index(string searchBy, string orderBy, string orderDir)
{
    var query = fca.GetResultsByFilter(searchBy);

    if (orderBy == "Campus" && orderDir == "Asc")
    {
        query = query = query.OrderBy(s => s.Campus).ThenBy(s => s.Student_Name);
    }
    else if (orderBy == "Campus" && orderDir == "Desc") 
    {
    query = query.OrderByDescending(s => s.Campus);
    }
    else if (orderBy == "Student Name" && orderDir == "Asc")
    {
        query = query = query.OrderBy(s => s.Student_Name);
    }
    else if (orderBy == "Student Name" && orderDir == "Desc")
    {
        query = query.OrderByDescending(s => s.Student_Name);
    }
    else if (orderBy == "Course Count" && orderDir == "Asc")
    {
    query = query.OrderBy(s => s.Course_Count);
    }
    else if (orderBy == "Course Count" && orderDir == "Desc")
    {
    query = query.OrderByDescending(s => s.Course_Count);
    }
}
Phil Ross
  • 25,590
  • 9
  • 67
  • 77
  • 1
    What is the `/* ... code ... */`? You can use LINQ to perform queries like this, assuming that is what is inside the code. – Cyral Jun 20 '15 at 23:27
  • I was speaking in a more generic sense. These are parameters send into the function by a form submit. I'll amend my posted question to provide that context. –  Jun 20 '15 at 23:28
  • You could at first create a method like If(check("campus","ASC"))..., then add your strings into an array and loop through...for(i...) {if(check(strby[i],strdir[i])) }....or create a special object for it if you don't like two arrays... – ElDuderino Jun 20 '15 at 23:38

4 Answers4

4

You could create an extension method on IQueryable that handles ordering with either OrderBy or OrderByDescending:

public static class QueryableExtensions
{
    public static IOrderedQueryable<TSource> OrderByWithDirection<TSource,TKey>
        (this IQueryable<TSource> source,
        Expression<Func<TSource, TKey>> keySelector,
        string orderDir)
    {
        return orderDir == "Desc" 
                        ? source.OrderByDescending(keySelector)
                        : source.OrderBy(keySelector);
    }
}

I'm assuming that your GetResultsByFilter method is returning an IQueryable<>. If it actually returns an IEnumerable<>, then the extension method will need to take an IEnumerable<TSource> source parameter and return an IOrderedEnumerable<TSource> instead.

This can then be used as follows:

public ActionResult Index(string searchBy, string orderBy, string orderDir)
{
    var query = fca.GetResultsByFilter(searchBy);

    switch (orderBy)
    {
        case "Campus":
            query = query.OrderByWithDirection(s => s.Campus, orderDir);
            break;
        case "Student Name":
            query = query.OrderByWithDirection(s => s.Student_Name, orderDir);
            break;
        case "Course Count":
            query = query.OrderByWithDirection(s => s.Course_Count, orderDir);
            break;
    }

    if (orderBy == "Campus" && orderDir == "Asc")
    {
        // The Campus Asc case was also ordered by Student_Name in the question.
        query = query.ThenBy(s => s.Student_Name);
    }
}
Phil Ross
  • 25,590
  • 9
  • 67
  • 77
  • Good question GetResultsByFilter(string) calls `IEnumerable GetResultsByFilter(string filter)`. Would you recommend putting this in the model or in the controller? –  Jun 21 '15 at 00:05
  • @Rubix_Revenge Assuming that this is a database query, it would be better to make `GetResultsByFilter` return `IQueryable`. This will allow the `OrderBy` to be executed at the database level as an SQL `ORDER BY` (see http://stackoverflow.com/q/2876616). I'd probably put the `GetResultsByFilter` method in the controller unless it was used in more than one controller. – Phil Ross Jun 21 '15 at 00:14
  • I made IEnumerable in order to this to work with the greater needs of the MVC View. You raise a good point, but now I'm re-entering the world of whack-a-mole, where making one change causes problems in other places. –  Jun 21 '15 at 00:19
  • @Rubix_Revenge `IQueryable` extends `IEnumerable`, so you should be able to use an instance that implements `IQueryable` anywhere that requires an `IEnumerable`. – Phil Ross Jun 21 '15 at 00:25
2

CNot sure if this is better, just different.

switch (orderDir)
{
    case "Asc":
        Switch (orderBy)
        {
            case "Campus":
                //Code here for Campus orderBy and Asc orderDir
                break;
            case "Student Name":
                //Code here for Student Name orderBy and Asc orderDir
                break;
            case "Course Count":
                //Code here for Course Count orderBy and Asc orderDir
                break;
        }
        break;
    case "Desc":
        Switch (orderBy)
        {
            case "Campus":
                //Code here for Campus orderBy and Desc orderDir
                break;
            case "Student Name":
                //Code here for Student Name orderBy and Desc orderDir
                break;
            case "Course Count":
                //Code here for Course Count orderBy and Desc orderDir
                break;
        }
        break;
}
DiscipleMichael
  • 510
  • 2
  • 17
1

I would use the terniary operator to make in more compact and easier to read like this.

This will also cut out some of the boolean checking as it doesn't duplicate any of them.

    public ActionResult Index(string searchBy, string orderBy, string orderDir)
    {
        var query = fca.GetResultsByFilter(searchBy);

        if (orderBy == "Campus")
        {
            query = (orderDir == "Asc") ? query.OrderBy(s => s.Campus).ThenBy(s => s.Student_Name) :
                query.OrderByDescending(s => s.Campus);
        }
        else if (orderBy == "Student Name")
        {
            query = (orderDir == "Asc") ? query.OrderBy(s => s.Student_Name) : query.OrderByDescending(s => s.Student_Name);
        }
        else if (orderBy == "Course Count")
        {
            query = (orderDir == "Asc") ? query.OrderBy(s => s.Student_Name) : query.OrderByDescending(s => s.Course_Count);
        }
    }
deathismyfriend
  • 2,182
  • 2
  • 18
  • 25
0

My take :

public interface IOrder {
    void perform(Query query)
}

public abstract class AbstractOrder : IOrder {

    protected string orderString;

    public AbstractOrder(string orderString) {
         this.orderString = orderString;
    }
}

public class OrderAsc {

    public OrderAsc(string orderString) : base(orderString) {
    }

    public Query perform(Query query) {
        query = query.OrderBy(s => s.Course_Count); //here you still have to do a mapping between orderString and your db field s.Course_count 
        return query;
    }
}

public class OrderDesc {

    public OrderDesc(string orderString) : base(orderString) {
    }

    public Query perform(Query query) {
        query = query.OrderByDescending(s => s.Course_Count); //here you still have to do a mapping between orderString and your db field s.Course_count, or maybe it's equal, then you can just replace it.
        return query;
    }
}

then ...

IList<IOrder> list = new List<IOrder>() {new OrderAsc("Campus"), new OrderDesc("Student Name")}

foreach(IOrder o in list) {
    query = o.perform(query);
}

There could be some mistakes in it, I don't have an IDE at hand atm.

ElDuderino
  • 3,253
  • 2
  • 21
  • 38
  • As I'm still on the language learning curve, this is certainly new to me. But this gives me some more material to chew on. Thanks –  Jun 21 '15 at 00:07