Can it be done more cleaner and shorter?

Question

I have a code which is in my opinion bad, because it has the similar statement repeated in each case statement:

if (command.OrderProperty.ToLower().Equals("asc"))
{
   accessForms.Forms = accessForms.Forms.OrderBy(o => o.NumberForSort).ToList();
}
else
{
   accessForms.Forms = accessForms.Forms.OrderByDescending(o => o.NumberForSort).ToList();
}

The only thing which is different is the property on which the query is ordered.

switch (command.SortProperty.ToLower())
{
   case "number":
      if (command.OrderProperty.ToLower().Equals("asc"))
      {
         accessForms.Forms = accessForms.Forms.OrderBy(o => o.NumberForSort).ToList();
      }
      else
      {
         accessForms.Forms = accessForms.Forms.OrderByDescending(o => o.NumberForSort).ToList();
      }
      break;
   case "type":
      if (command.OrderProperty.ToLower().Equals("asc"))
      {
         accessForms.Forms = accessForms.Forms.OrderBy(o => o.Type).ToList();
      }
      else
      {
         accessForms.Forms = accessForms.Forms.OrderByDescending(o => o.Type).ToList();
      }
      break;
   case "employeename":
      if (command.OrderProperty.ToLower().Equals("asc"))
      {
         accessForms.Forms = accessForms.Forms.OrderBy(o => o.EmployeeName).ToList();
      }
      else
      {
         accessForms.Forms = accessForms.Forms.OrderByDescending(o => o.EmployeeName).ToList();
      }
      break;
   case "requestingemployeename":
         (...)

}

accessForms.Forms is a List<>. I would like to make this code more cleaner and shorter, but I have no idea how to achieve that.

This is more suited for https://codereview.stackexchange.com/ as you have working code. But I think you can at least create lamba expressions in the switch, and handle the sort order outside the switch. — Wai Ha Lee, Jan 16 '19 at 12:26
You should not use TLower but ToUppad - .NET is optimized for upper comparison, not lower. https://learn.microsoft.com/en-us/dotnet/standard/base-types/best-practices-strings — TomTom, Jan 16 '19 at 12:39
@TomTom You shouldn't use either. You should use a case insensitive comparer if you want to compare strings in a case insensitive way. — Servy, Jan 16 '19 at 15:53

score 1 · Accepted Answer · answered Jan 16 '19 at 12:32

The following should work, basically building a func extracting the sort field from a form and using it in the Linq operations.

Note that you could use a more strongly typed func, if the sort fields were all strings for instance.

Func<Form, object> sortFieldGetter = null;

switch (command.SortProperty.ToLower())
{
   case "number":
       sortFieldGetter = form => (object)form.NumberForSort;
       break;

   case "type":
       sortFieldGetter = form => (object)form.Type;
       break;

   case "employeename":
       sortFieldGetter = form => (object)form.EmployeeName;
       break;

   ...
}

if (command.OrderProperty.ToLower().Equals("asc"))
{
    accessForms.Forms = accessForms.Forms.OrderBy(sortFieldGetter).ToList();
}
else
{
    accessForms.Forms = accessForms.Forms.OrderByDescending(sortFieldGetter).ToList();
}

score 0 · Answer 2 · answered Jan 16 '19 at 12:40

Using @rob code written here, in your case the final code can look like this:

 accessForms.Forms = command.OrderProperty.ToLower().Equals("asc")) ?
     accessForms.Forms.OrderBy(command.SortProperty.ToLower()).ToList() :
     accessForms.Forms.OrderByDescending(command.SortProperty.ToLower()).ToList();

Note that you need to implement your own OrderByDescending that isn't in the original code.

Alexandru Clonțea · Answer 3 · 2019-01-16T15:53:51.320

I agree this question is perhaps better suited for Code Review.

In any case, you can do something along these lines:

Step 1: Create extension method(s)

public static IQueryable<T> SortByField<T, TKey>(this IQueryable<T> source, Expression<Func<T, TKey>> selector, bool ascending = true)
{
    return ascending ? source.OrderBy(selector) : source.OrderByDescending(selector);
}

public static IQueryable<T> SortByField<T, TKey>(this IQueryable<T> source, Expression<Func<T, TKey>> selector, string order = "asc")
{
    return SortByField(source, selector, order.Equals("asc", StringComparison.OrdinalIgnoreCase)); //condition here can be improved, just as a sample of how to do it
}

Step 2: Enjoy:

 var samples = new List<Sample>().AsQueryable(); //you can of course use LINQ to SQL or EF, but I am lazy
 samples.SortByField(t => t.NumberForSort, false); //or "asc"

I think improving your "command"/query object would help clean up the code.

Rant: "Case" is for me a last resort, it almost always means that there are at least two places where I must change code, which might not always be located in the same assembly, and almost all uses of "case" can be replaced with Dictionary or the Strategy pattern.

Can it be done more cleaner and shorter?

3 Answers3