0

I inherited a project, and asked to implemented Paging across all our Repositories (500+ methods), to show only segments of data on web front end.

Current Repository is below as following. They return Task Lists, IEnumerables etc .

How would we go through, and implement paging for the following Repositories examples?

If I implement following Paged List answer, the repos return IList, not IQueryable. So the answer, would still Retrieve All the Data first (causing performance issue), and then filter it.

https://stackoverflow.com/a/5036023/12425844

Repository:

public Task<List<Product>> GetProductListByProductNumber(int bkProductNumber)
{
    var productData = _context.Product
        .Include(c => c.LkProducttype)
        .Where(c => c.BkProductnumber == bkProductNumber).ToListAsync();
    return productData ;
}


public Task<List<Product>> GetProductListByProductDescription(string productDescription)
{
    var productData = _context.Product
        .Include(c => c.LkProducttype)
        .Where(c => c.ProductDescription == productDescription).ToListAsync();
    return productData;
}

Page List Answer

public PagedList<T> Find(Expression<Func<T,bool>> predicate, int pageNumber, pageSize)
{
   return repository
             .Find()
             .Where(predicate)
             .ToPagedList(pageNumber, pageSize);
}

*Looking for an efficient way, so don't have to Recopy and Paste All the 500+ methods with paging,if possible

If changing all the Repos methods to IQueryable, that may fix the issue. However, was reading Repos should Not return IQueryable per article here Entity Framework Repository Pattern why not return Iqueryable?

  • You can replace the call to ToListAsync() with ToPagedList() – Enas Osama Jun 23 '20 at 16:54
  • hi @EnasOsama then what do I do with original repositories, which do not require paging? I still need them too, thanks –  Jun 23 '20 at 16:55
  • To handle paging the returned object would have to differ to hold information about total count and current page. how do you plan on handling that concern so that I can try to help :) – Enas Osama Jun 23 '20 at 17:07
  • maybe place a parameter as repository name, looking for any solution, trying to understand myself? @EnasOsama –  Jun 23 '20 at 17:12
  • Well, the only thing I can think of now is maybe you can edit the .ToPagedList() extension method to handle this. it can take optional pagination parameters. if present use the paginated version, else use the regular ToListAsync(). the only problem is that you have to have a wrapper object to handle both cases or use dynamic as a return object – Enas Osama Jun 23 '20 at 17:37
  • You'd have to edit the 500+ methods but based on the snippet this can be a find and replace task. – Enas Osama Jun 23 '20 at 17:38
  • Whether or not repositories should return `IQueryable` is totally your own decision. If they're not directly exposed, go for it. – Gert Arnold Jun 23 '20 at 19:00

1 Answers1

0

If you want to return exactly the data for a page you can use LINQ .Skip and .Take, that will still return a IQueryable

public Task<List<Product>> GetProductListByProductNumberAsync(int bkProductNumber, int pageNumber, int pageSize)
{
    var productData = _context.Product
        .Include(c => c.LkProducttype)
        .Where(c => c.BkProductnumber == bkProductNumber)
        .Skip(pageSize * (pageNumber - 1))
        .Take(pageSize)
        .ToListAsync();
    return productData;
}

The default of this approach is that if new data appears regularly, your pages can lose synchronisation. For example, your query page 2, 25 items and get 25-49. Then 2 new items are inserted and you query page 3, you will get what used to be 48-72 But this is a standard behaviour on most web sites.

Now to address the issue of DRY if you want to keep the existing queries, I would separate first the query itself from the transformation to an asynchronous list:

  private IQueryable<Product> GetProductListByProductNumberQuery(int bkProductNumber)
        {
            var productData = _context.Product
                .Include(c => c.LkProducttype)
                .Where(c => c.BkProductnumber == bkProductNumber);
            return productData;
        }

And then add the way to use the queryable as needed (example with extension methods, but can be done with a class to wrap, or in your repository with 2 ways of invoking a queryable) :

  public static class QueryableExtensions
    {
        public static Task<List<T>> ToTask<T>(this IQueryable<T> query) 
        {
            return query.ToListAsync(); // might as well call directly .ToListAsync
        }

        public static Task<List<T>> ToTaskPaged<T>(this IQueryable<T> query, int pageNumber, int pageSize)
        {
            return query
                .Skip(pageSize * (pageNumber-1))
                .Take(pageSize)
                .ToListAsync();
        }
    }

Differing the use of ToListAsync allow to come back to the query to refine it with paging

Whether the queries are public or private will depend on how you want to use them. If you consider that the repository is responsible for providing the queries but not how to run them, then the queries will be public. If you want your repository to only expose wrapped executed queries, then the queries will be private. Good practice is to not return IQueryable, but that would run against the constraint you set that you do not want to create 2 versions of each query (paged, not paged), so with that constraint I would go for exposing the IQueryable. But the articles you link are right in warning that users can start doing insane things with it. If it is exposed to your team only, I think it's fine as long as every one is aware that it is a technical debt. If it is in a public API, then you can't afford to fix it later.

Shaiar
  • 91
  • 7
  • If refined for the DRY concern (totally legitimate, I didn't get you wanted to keep the former ones too), but nothing cross my mind about how to avoid editing 500 methods as the people before you wrapped together the query itself and its use, so you don't have access to the IQueryable anymore – Shaiar Jun 23 '20 at 17:38
  • Refined a bit more following your concern about public IQueryable, but to make one thing clear: I do not recommend to expose IQueryable normally. It's to work with your constraint, which I assume is related to the time you have for the task. Otherwise I would bite the bullet and make 2 public methods, one with and one without paging, both using the same private queryable generator. But that is 500 methods to add. – Shaiar Jun 24 '20 at 16:44
  • yep, thats why I am making IQueryable Private, and then the List Enumerable Queries Public, your strategy makes sense ! –  Jun 24 '20 at 16:46
  • There is also a solution with using default parameters for a pagingOptions parameter, but that is also not a good design. But we can look into that if you really want to avoid the 500 extra methods and still not return IQueryable. – Shaiar Jun 24 '20 at 16:47
  • yeah, would be happy to alook at alternative –  Jun 24 '20 at 16:56
  • Sorry I've been a bit swamped and didn't answer again here. The alternative I had in mind bring no benefit, so it's not worth looking more into it. I thought about the suggestion made by @Enas Osama in comments and there are a couple hacks that could work but all I could come up with was quite disgusting to be honest :) – Shaiar Jun 28 '20 at 03:25