To return IQueryable<T> or not return IQueryable<T> [closed]

I have a repository class that wraps my LINQ to SQL Data Context. The repository class is a business line class that contains all the data tier logic (and caching and such).

Here's my v1 of my repo interface.

public interface ILocationRepository
{
    IList<Location> FindAll();
    IList<Location> FindForState(State state);
    IList<Location> FindForPostCode(string postCode);
}

But to handle paging for FindAll, I'm debating whether or not to expose IQueryable<ILocation> instead of IList to simplify the interface for circumstances such as paging.

What are the pros and cons to exposing IQueryable from the data repo?

Any help is very much appreciated.

Solution 1:

The pros; composability:

• callers can add filters
• callers can add paging
• callers can add sorting
• etc

The cons; non-testability:

• Your repository is no longer properly unit testable; you can't rely on a: it working, b: what it does;
  • the caller could add a non-translatable function (i.e. no TSQL mapping; breaks at runtime)
  • the caller could add a filter/sort that makes it perform like a dog
• Since callers expect IQueryable<T> to be composable, it rules out non-composable implementations - or it forces you to write your own query provider for them
• it means you can't optimize / profile the DAL

For stability, I've taken to not exposing IQueryable<T> or Expression<...> on my repositories. This means I know how the repository behaves, and my upper layers can use mocks without worrying "does the actual repository support this?" (forcing integration tests).

I still use IQueryable<T> etc inside the repository - but not over the boundary. I posted some more thoughts on this theme here. It is just as easy to put paging parameters on the repository interface. You can even use extension methods (on the interface) to add optional paging parameters, so that the concrete classes only have 1 method to implement, but there may be 2 or 3 overloads available to the caller.

Solution 2:

As mentioned by previous answer, exposing IQueryable gives access to callers to play with IQueryable itself, which is or it can become dangerous.

Encapsulating business logic's first responsibility is to maintain integrity of your database.

You can continue exposing IList and may be change your parameters as following, this is how we are doing...

public interface ILocationRepository
{
    IList<Location> FindAll(int start, int size);
    IList<Location> FindForState(State state, int start, int size);
    IList<Location> FindForPostCode(string postCode, int start, int size);
}

if size == -1 then return all...

Alternative way...

If you still want to return IQueryable, then you can return IQueryable of List inside your functions.. for example...

public class MyRepository
{
    IQueryable<Location> FindAll()
    {
        List<Location> myLocations = ....;
        return myLocations.AsQueryable<Location>;
        // here Query can only be applied on this
        // subset, not directly to the database
    }
}

First method has an advantage over memory, because you will return less data instead of all.

Solution 3:

I recommend using IEnumerable instead of IList, with it you will have more flexibility.

This way you will be able to get from Db only that portion of data which you really gonna use without extra work done in your repository.

Sample:

// Repository
public interface IRepository
{
    IEnumerable<Location> GetLocations();
}

// Controller
public ActionResult Locations(int? page)
{
    return View(repository.GetLocations().AsPagination(page ?? 1, 10);
}

Which is super clean and simple.