views:

3050

answers:

3

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.

+1  A: 

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.

Koistya Navin
Why? What are the trade-offs?
Richard
IList<Location> FindAll() is not very efficient if you're going to display a paged list to the user (let's say 1000 rows will be fetched from db, but only 10 of them will be shown). With IQueryable you won't have such an issue.
Koistya Navin
BTW, if you're going to implement paging functionality take a look at existing helper classs in MvcContrib = http://www.codeplex.com/mvccontrib/ (MvcContrib.Pagination namespace)
Koistya Navin
That requirement could be exposed just as easily on the repository interface...
Marc Gravell
+22  A: 

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.

Marc Gravell
@Marc, what would you tell about IEnumerable<T> for repository methods?
Koistya Navin
If used appropriately, `IEnumerable<T>` would be OK - but mainly for large data. For regular queries, I'd prefer a closed set (array/list/etc). In particular, I'm using MVC at the moment, and I want the **controller** to fetch all data - not defer it until the view - else you can't fully test...
Marc Gravell
...the controller, as you haven't actually proven it fetching data (since `IEnumerable<T>` is usually used with deferred execution). If the repo returns IList<T> (or similar), then you **know** you have got the data.
Marc Gravell
@Mark, and.. what's wrong with firing an executin within unit test? Ex.: `Assert.IsTrue(result.Any())`
Koistya Navin
With IQueryable<T>, that changes the actual query. With IEnumerable<T>, less so - but still: getting data is the responsibility of the controller, not the view.
Marc Gravell
+3  A: 

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.

Akash Kava
This is not so elegant. This way CVertex must add those parameters (start, size) to EVERY repository method he creates - very bad programming practice.
twk
Well its better then exposing Data Base Interface which can modify and spoil your data.
Akash Kava
Only needs to add the paging parameters where they're actually used. And if they're used, its not wasted effort.
Frank Schwieterman