Is it good practice to have linq query in Controllers?

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?

I'm sure opinions may vary, but I've learned to try to keep as much business logic in the service as you can. 3 would be my choice. With 1, you've already spotted the issue. With 2, you are introducing display precedence in a service. With 3, you handle display preferences where necessary. If you were to introduce another interface to your business layer, you are requiring potentially, unnecessary code iterations by choosing 2.

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.