views:

92

answers:

3

I'm trying to make a repository class that has a method to order the results based upon a "sort" parameter. I need to pass it as a parameter since I'm trying to be very strict that my repository doesn't return IQueryable and only returns List. The problem is that I have no idea how to make it so that it meets the following requirements:

  1. Allow for multiple columns.
  2. Strongly typed to the entity that's returned (no strings for columns as parameters).
  3. Ability to set a specific column as descending.

Is this even possible or is it useless that a Repository allows returning with ordering? Should the repository be just able to perform CRUD operations? Maybe returning IQueryable would be the best option?

+3  A: 

This is arguable, but I think your repository should just return the data. Let your consuming classes worry about the ordering of the set.

The thought process behind this is pretty simple, the consuming classes decide the order anyway. So they can pass that to hand crafted orderby generator in your repository and let it do the work, or they can just use linq and order the set they get back from the repository. I think the latter option is easier to implement, less prone to bugs, and will make more sense when reading the code, but once again, I think my point is arguable.

Matthew Vines
Without getting argumentative, I agree with your point most of the time, but there are a few times when it can be useful to allow the repository to handle the ordering. For instance, when the actual data store can sort much more efficiently than your code can, or if you need to offload processing to keep the current system responsive.
NickLarsen
I agree with you that it'd make more sense. But shouldn't all execution of the SQL (aka .ToList() ) take place in the Repository and not further down the road?
TheCloudlessSky
@TheCloudlessSky: Why? That's really the point of IQueryable<T> - you're allowing the data execution to be deferred until it's really needed, which means you can prevent unnecessarily complex queries from executing against your DB. Making your repository do ToList() just means you're putting more burden on your DB...
Reed Copsey
@Reed or the opposite can be true as well. Since your query can be modified outside of the repository, a query could be constructed outside of the repository which has the same effect. If you keep your access inside the repository, you can eliminate that issue. I do see plenty of use for both solutions however.
NickLarsen
+2  A: 

I would argue that returning the IQueryable<T> directly is typically the best option.

For example, say you have a method that queries your DB to return a specific table or view. If the client of your repository only needs the first 10 records, with custom criteria and a sort you weren't expecting, you can allow this by returning IQueryable<T>. The client just needs to add the .Where(...).OrderBy(...).Take(10);

With IQueryable<T>, the call across the data layer will automatically adapt, and only pull the 10 records across. If you return a List instead, your DB query will pull every record, and then the filtering will need to happen in your application.

This adds a huge processing/network/etc overhead, with no real reason.

Reed Copsey
This is my dilemma with this. From what I've read, it's best *not* to return `IQueryable<T>` for repositories.
TheCloudlessSky
@TheCloudlessSky: I don't know where you've read that, but I'd argue that it is bad advice... unless there is some specific reason to prevent this.
Reed Copsey
The first thing that came to mind about this is that you are moving the actual data access out of the repository and to an arbitrary location using this technique. It does solve the problems listed however.
NickLarsen
See: http://stackoverflow.com/questions/718624/to-return-iqueryablet-or-not-return-iqueryablet for my reasons NOT to return IQueryable.
TheCloudlessSky
+1  A: 

Perhaps you need something like this:

public class Ordering<T>
{
    private readonly Func<IQueryable<T>, IOrderedQueryable<T>> transform;

    private Ordering(Func<IQueryable<T>, IOrderedQueryable<T>> transform)
    {
        this.transform = transform;
    }

    public static Ordering<T> Create<TKey>
        (Expression<Func<T, TKey>> primary)
    {
        return new Ordering<T>(query => query.OrderBy(primary));
    }

    public Ordering<T> ThenBy<TKey>(Expression<Func<T, TKey>> secondary)
    {
        return new Ordering<T>(query => transform(query).ThenBy(secondary));
    }

    // And more for the descending methods...

    internal IOrderedQueryable<T> Apply(IQueryable<T> query)
    {
        return transform(query);
    }
}

Then a client can create an Ordering<T> to be passed into the repository, and the repositry can call Apply with the IQueryable<T>. Does that make sense?

Sample (slightly silly) use:

var ordering = Ordering<FileInfo>.Create(fi => fi.Length)
                                 .ThenBy(fi => fi.Name);
Jon Skeet
Yeah that's pretty much exactly what I was looking for. How can I use this without having to supply the types from`repository.GetOrderedBy(Ordering<EntityType>.Create<EntityType, int>(e => e.Column))`to using type inference: `repository.GetOrderedBy(Ordering.Create(e => e.Column))`Or is this not possible since `Ordering.Create` is static?
TheCloudlessSky
@TheCloudlessSky: Whoops, that's a typo. The Create method should only declare one type parameter.
Jon Skeet
Thanks Jon, makes sense now. Do you think doing this is overkill? Do you think it'd just be best to return `IQueryable` and do the ordering outside the repository?
TheCloudlessSky
@TheCloudlessSky: Well, I know I read an article recently saying how evil it is to return `IQueryable<T>` from your data layer. I certainly think there are pros and cons on each side... and if you *do* want to avoid doing that, this is a reasonable way of representing an ordering. It's nicely reusable too (i.e. you don't need to create a new one each time) :)
Jon Skeet