Is it good practice to have linq query in Controllers?

Question

I'm not very familiar with the MVC pattern. Could you tell me which one of the following three controller actions is better? Thanks :)

(1) Have query in action:

public ActionResult List()
{
   var query = repository.Query().Where(it => it.IsHandled).OrderBy(it => it.Id);
   // ...
}

(2) Have query in service:

public ActionResult List() 
{
    var items = service.GetHandledItemsOrderById();
    // ...
}

(3) Have order by in action:

public ActionResult List()
{
    var items = service.GetHandledItems().OrderBy(it => it.Id);
    // ...
}

If we choose (1), then we have too much business logic in controller?

If we choose (2), there might be lots of service methods like GetXXXByYYY().

If we choose (3), why we encapsulate Where(it => it.IsHandled) but not
OrderBy(it => it.Id.

Any ideas?

Answer 1

It depends. :)

My opinion:

I like to keep my service loose, to minimize duplicate code. I'm also a fan of pipes and filters.

Here's what i'd do (and DO do).

Service

public ICollection<Item> GetHandledItems<TKey>(OrderingOptions<Item, TKey> orderingOptions) 
{
   return repository
      .Query()
      .WhereHandled()
      .WithOrdering(orderingOptions)
      .ToList();     
}

ItemFilters.cs

public static IQueryable<Item> WhereHandled(this IQueryable<Item> source)
{
   return source.Where(it => it.IsHandled);
}

public static IOrderedQueryable<T> WithOrdering<T, TKey>(
   this IQueryable<T> source,
   OrderingOptions<T, TKey> orderingOptions)
{
   return orderingOptions.SortDescending 
      ? source.OrderByDescending(orderingOptions.OrderingKey) :                                                    
        source.OrderBy(orderingOptions.OrderingKey);
}

OrderingOptions.cs

 public class OrderingOptions<T,TKey>
 {
    public OrderingOptions(Expression<Func<T,TKey>> orderingKey, bool sortDescending = false)
    {
       OrderingKey = orderingKey;
       SortDescending = sortDescending;
    }

    public Expression<Func<T,TKey>> OrderingKey { get; private set; }
    public bool SortDescending { get; private set; }
 }

This way, you can specify the ordering in the Controller:

var items = service.GetHandledItems(new OrderingOptions(it => it.Id));

Differences between the above and options 3:

• Above materializes sequence before returning to Controller. Option 3 does not, which is dangerous (you could end up returning query to View and break MVC pattern).
• Generic "Ordering" POCO, can be used anywhere and keeps your queries D-R-Y.
• Service becomes dumb, and simply a mitigator between the Repository and the Controller (which is all it should do, IMO). Logic (e.g filters) abstracted to one place.