Include OrderBy delegate in Method parameters

Question

I have a method;

    public List<Task> GetTasksByAssignedTo(Guid contactId)
    {
        List<Task> tasks = dc.Tasks.Where(x => x.ContactId == contactId).ToList();
        return tasks;
    }

which returns a list of items. Say I now wanted to specify the sort order I want to return the list in.

So I might sort by Name, DueDate, Completed etc, etc.

How could I include that in the method as a parameter? I don't want to use a switch statement rather I'd like to use a lambda if possible.

So;

List<Task> list = GetTasksByAssignedTo("guid", ??????);

Or is this the wrong approach.

Answer 1

I think that your approach is the wrong way to use LINQ.

LINQ uses a deferred execution model for a reason. It allows you to chain together a series of operations that get executed only when you tell it to compute the result - often with .ToList(), .ToArray(), .First() - but you can also force the computation by filtering with a OrderBy clause that uses a Func<T, ?> as its parameter.

Now you're returning a List<Task> which means that you've forced the execution - which is the right thing to do when you're ready to use the results - but if you then go on to do further operations you are potentially loading many more records into memory than you need to.

You could certainly do this:

public List<Task> GetTasksByAssignedTo<P>(Guid contactId, Func<Task, P> orderBy)
{
    return dc.Tasks
        .Where(x => x.ContactId == contactId)
        .OrderBy(orderBy) // this forces evaluation - sort happens in memory
        .ToList();
}

To make the execution happen in the database you need to change it like this:

public List<Task> GetTasksByAssignedTo<P>(
    Guid contactId,
    Expression<Func<Task, P>> orderBy)
{
    return dc.Tasks
        .Where(x => x.ContactId == contactId)
        .OrderBy(orderBy)
        .ToList(); // Now execution happens here
}

But the issue is what if you did this:

var query =
    from t1 in GetTasksByAssignedTo(contactId, t => t.Name)
    join t2 in GetTasksByAssignedTo(contactId, t => t.Name)
        on t1.Name equals t2.Name
    select new { t1, t2 };

Because your GetTasksByAssignedTo brings records into memory you are doing the join in memory. (Yes, the query is a bit contrived, but the principle is solid though.)

It's often much better to do it in the database.

Here's how to fix it:

public IQueryable<Task> GetTasksByAssignedTo<P>(
    Guid contactId,
    Expression<Func<Task, P>> orderBy)
{
    return dc.Tasks
        .Where(x => x.ContactId == contactId)
        .OrderBy(orderBy);
}

Now the above query won't execute until you do query.ToList() and all will happen at the database.

But I have an even bigger issue.

You're hiding a lot of information in the GetTasksByAssignedTo. Someone using the code doesn't know that they're actually getting a list when they read the code and they really don't know if the actual implementation is doing the right thing. I think, for these kinds of queries, it's often better to leave it as plain LINQ.

Compare these:

var tasks1 = GetTasksByAssignedTo(contactId);
var tasks2 = GetTasksByAssignedTo(contactId, t => t.Name);
var tasks3 = GetTasksByAssignedToDescending(contactId, t => t.Name);

var tasks4 = (
        from t in dc.Tasks
        where t.ContactId == contactId
        orderby t.Name descending
        select t
    ).ToList();

The first query, tasks1 isn't too bad, but it doesn't tell you what the return type is;

The second query, tasks2 does something with some t and the property Name, but doesn't tell you what.

The third query, tasks3 give you a hint that it is sorting descending, but doesn't tell you if it's by the mysterious Name property or something else.

The fourth query, tasks4 tells you everything that you need to know - it's filtering tasks by ContactId, reverse ordering the results by Name, and finally returning a list.

Now, take a look at this query:

var query2 =
    from t1 in dc.Tasks
    where t1.ContactId == contactId
    join t2 in dc.Tasks on t1.Name equals t2.Name
    where t2.ContactId != contactId
    orderby t2.Name descending
    select t2;

I can read that quite easily and see what it is doing. Just imagine what the helper method name would be for this one! Or what insane nesting of helper methods would be required.

The bottom-line is that LINQ is the API for querying.

If you desperately want to create helper methods then use extension methods.

public static class TaskEx
{
    public static IQueryable<Task> WhereAssignedTo(this IQueryable<Task> tasks,
        Guid contactId)
    {
        return tasks.Where(t => t.ContactId == contactId);
    }

    public static IQueryable<Task> OrderByName(this IQueryable<Task> tasks)
    {
        return tasks.OrderBy(t => t.Name);
    }
}

This then allows you to write this:

var tasks = dc.Tasks
    .WhereAssignedTo(contactId)
    .OrderByName()
    .ToList();

And that is clear, concise, extensible, composable, re-usable, and you control when execution occurs.

Answer 2

You could pass a Func<Task, object> to your method for ordering:

public List<Task> GetTasksByAssignedTo(Guid contactId, Func<Task, object> someOrder)
{
    List<Task> tasks = dc.Tasks.Where(x => x.ContactId == contactId)
                               .OrderBy(someOrder)
                               .ToList();
    return tasks;
}

Now you can call your method like

Func<Task, object> someOrder = (Task t) => t.DueDate;
List<Task> list = GetTasksByAssignedTo(someGuid, someOrder);

Generally I agree with the comments though - it does not seem that ordering is required for a method named GetTasksByAssignedTo.