Keep Your IQueryable In Check

Writing

Updated: I’ve posted a follow-up on this post here.

There is a good chance that you have seen Oren’s recent “Repository Is The New Singleton” post. I wanted to have a quick discussion of his approach, and discuss why I don’t 100% agree with it. Let me first say that I don’t have a problem with the repository pattern. I do have some issue with the way that it is implemented sometimes, but overall I think it provides a good layer in which to encapsulate the queries of your application.

On that note, I do see value in the pattern which Oren refers to as the Query Object pattern. Some people may look at the pattern Oren is using and see “class explosion”, but I think that it all depends on what your preferences are. I think that the repository method can be implemented in most medium sized application without much trouble, but when you get to really large applications, you can get into trouble with lots of little queries that can create huge repositories. I’d much rather have one hundred query objects than to have repositories with a hundred methods.

Okay, so I agree with Oren that encapsulating queries into objects is not a bad idea, especially if your application has a large number of complex queries. My problem with Oren’s approach comes from the example that he gives of one of these “Query Objects” in a later post (shortened to save space):

public class LatePayingCustomerQuery
{
    /* snip */
    public IQueryable<Customer> GetQuery(ISession session)
    {
        var payments =     from payment in s.Linq<Payment>()
                     where payment.IsLate
                    select payment;
    
        /* snip */
        
        return     from customer in s.Linq<Customer>
                where payments.Any( payment => customer == payment.Customer )
                select customer;
    }
}

My problem is in returning IQueryable from the “GetQuery” method. Oren does it in order to provide the ability to sort, page, etc… closer to the UI layer, and I agree that this is necessary, but I think that the result needs to be constrained a bit more. Passing back the IQueryable allows much more than just sorting and paging, it really allows heavy modification of the query itself, and in a way that you are really no longer able to control. So it seems to me that Oren’s problem really isn’t so much with the repository pattern as it is with the idea of completely encapsulating all interaction with the ORM inside of the repository. He thinks that this causes us to lose much of the power and flexibility that ORMs provide us. But why can’t there be a middle ground?

So let’s assume that we have a fairly large application with a bunch of these finely crafted query objects, and you have a developer who has the above query, but decides that they just need to add one more filter to it. Now I know that people will say that if you have a developer who breaks the rules then you probably don’t want them to work on your team, but this is the real world. And in the real world people cut corners. So they do something like this:

latePayingCustomQuery
  .Where(c => c.LastName.StartsWith("E"))
  .Skip((page - 1) * itemCount)
  .Take(10);

This developer may have known better, or maybe they didn’t, but what they did was change the query so that we are now potentially filtering against a non-indexed string column in the database. And there is really no limit to what the developer could do to the query. I can already hear people screaming about bad developers and not wanting someone on the team who cuts corners, etc…. but I honestly don’t want to hear it. Quite often I don’t get to choose who I have to work with, especially when working with clients. So I don’t want to hear about your fairytale land where all developers do the right thing all the time.

So we really have two conflicting requirements, we want to provide the flexibility of paging and sorting up closer to the UI, but we also want to lock down the queries a bit. So what if we defined our own interfaces which we could use to wrap IQueryable? Something like this:

public interface IPageable<T>: IEnumerable<T>
{
    ISortable<T> Page(int page, int itemCount);
}

public interface ISortable<T>: IEnumerable<T>
{
    ISortable<T> OrderBy<U>(Expression<Func<T,U>> orderBy);
    ISortable<T> OrderByDescending<U>(Expression<Func<T,U>> orderBy);
}

This would allow us to have a result type of IPageable<T> which the developer could use like this:

query.FindAll().Page(1, 3).OrderBy(q => q.PostedDate);

Looks pretty good. The implementation of the IPageable interface could look something like this:

public class Pageable<T>: IPageable<T>
{
    private readonly IQueryable<T> queryable;

    public Pageable(IQueryable<T> queryable)
    {
        this.queryable = queryable;
    }

    public ISortable<T> Page(int page, int itemCount)
    {
        return new Sortable<T>(
            queryable
                .Skip((page - 1) * itemCount)
                .Take(itemCount));
    }

    public IEnumerator<T> GetEnumerator()
    {
        return queryable.GetEnumerator();
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }
}

Creating a pageable item would be as simple as just passing any IQueryable to it. Then all you have to do is pass a page number and an item count and it returns a sortable object. The sortable object would look something like this:

public class Sortable<T>: ISortable<T>
{
    private IQueryable<T> queryable;

    public Sortable(IQueryable<T> queryable)
    {
        this.queryable = queryable;
    }

    public ISortable<T> OrderBy<U>(Expression<Func<T,U>> orderBy)
    {
        queryable = queryable.OrderBy(orderBy);
        return this;
    }

    public ISortable<T> OrderByDescending<U>(Expression<Func<T,U>> orderBy)
    {
        queryable = queryable.OrderByDescending(orderBy);
        return this;
    }

    public IEnumerator<T> GetEnumerator()
    {
        return queryable.GetEnumerator();
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }
}

Here you can see that it allows us to chain sortable calls so that we can still order by multiple columns like this:

sortable.OrderBy(q => q.PostedDate).OrderBy(q => q.Title);

At this point you may be thinking that by exposing the “OrderBy” and “OrderByDescending” methods we have opened up the bad querying box just as much as passing back IQueryable. Well, with the code provided this is true…but if you use a bit of imagination you can also see how we could use these classes to limit the columns on which different types are allowed to be ordered by simply walking the expression tree when the enumerator is requested or intercepting the calls to “OrderBy” and “OrderByDescending”. It is also possible to create similar interfaces in this same way to provide constrained interfaces to other types of functionality such as projections.

Overall I think that ORMs have certainly revolutionized the way that developers think about data access, but I think we really need to be careful about letting IQueryable (or any other querying mechanism) flow up through our applications because all we are doing is letting spreading queries all over. I pretty much agree with Oren in his thoughts that the repository might not be the solution to everything, and that we need to think about data access in a slightly different way, but I also think that there is some serious need for a bit of control.

So what are your thoughts? What are the problems that you see in this approach? Do you feel like we should let IQueryable go throughout our applications? Do you think I am being too Byzantine? Let me know!

More Insights

View All