HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Paginating blog posts from SQL server using nested LINQ statements

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fromlinqserversqlstatementspaginatingnestedusingblogposts

Problem

I made an ASP.NET MVC application using Entity Framework for code-first migrations to the SQL Server database. I also use Unity (DI/IoC) for managing the services and repositories.

I made an overview page but I'll limit the items on that page to 10, 50 or 100 (variable named pagecount) items as selected by the user. The rest of the items are on other pages (variable named page). The result is something similar to what Code review has:

Each Blog has a Category. I'll also that the user can see one, more, or all categories (variable named categories). This contains a list of integers with the categories that the user will see.

using System.Data.Entity;

public class BlogRepo : GenericRepo
{
    public List GetByPage(int page, List categories, int pagecount)
    {
        return (from b1 in ((from b2 in ((from b3 in dbSet.Include(b3 => b3.Category).Include(b3 => b3.User)
                                          orderby b3.CreationDateStamp ascending
                                          where categories.Contains(b3.CategoryId)
                                          select b3).Take(page * pagecount)).Include(b2 => b2.Category).Include(b3 => b3.User) // step one
                             orderby b2.CreationDateStamp descending
                             select b2).Take(pagecount)).Include(b1 => b1.Category).Include(b3 => b3.User) // step two
                orderby b1.CreationDateStamp ascending
                select b1).ToList(); // step tree
    }
}


Here is my way of working:

-
I take the first items calculated by the formula page * pagecount sorted in ascending order by CreationDateStamp where categories.Contains(b3.CategoryId) is true.

-
Form that items I sort it in descending order on the same property, and take the value of pagecount.

-
Finally, sort it again (ascending), and call ToList() to materialize the results.

Note one: I'll always include the properties Category (type of Category) and User (type of `Ap

Solution

Your linq in linq in linq statement is written as a one-liner, with a single semicolon:

return (from b1 in ((from b2 in ((from b3 in dbSet.Include(b3 => b3.Category).Include(b3 => b3.User)
                                  orderby b3.CreationDateStamp ascending
                                  where categories.Contains(b3.CategoryId)
                                  select b3).Take(page * pagecount)).Include(b2 => b2.Category).Include(b3 => b3.User) // step one
                     orderby b2.CreationDateStamp descending
                     select b2).Take(pagecount)).Include(b1 => b1.Category).Include(b3 => b3.User) // step two
        orderby b1.CreationDateStamp ascending
        select b1).ToList(); // step tree


In other words, it's doing too many things - you even have comments to identify the steps!

LINQ queries don't execute until they're materialized - you can take advantage of that and split the complex query into smaller steps that don't iterate the results.

Step one, grab all items up to the end of the desired page - this could be in its own method:

private IEnumerable FetchItemsUpToSpecifiedPage(int page, int pageSize, IEnumerable categories)
{
    return (from blog in dbSet.Include(item => blog.Category)
                              .Include(item => blog.User)
            orderby blog.CreationDateStamp ascending
            where categories.Contains(blog.CategoryId)
            select blog)
           .Take(page * pageSize);
}


Notice the following:

  • pagecount is renamed to a more accurate pageSize; "page count" sounds like like the number of pages, but what you want it to convey is the number of items on a page.



  • Meaningful identifiers. blog makes for easier reading than b3.



  • The compiler infers the generic type argument of Take from the source parameter of the extension method, so specifying it is redundant and clutters up the code.



Step two, filter for specified page - again, in its own method:

private IEnumerable FilterForSpecifiedPage(int page, int pageSize, IEnumerable categories)
{
    var items = FetchItemsUpToSpecifiedPage(page, pageSize, categories);
    return (from blog in items.Include(item => blog.Category)
                              .Include(item => blog.User)
            orderby blog.CreationDateStamp descending
            select blog)
           .Take(pageSize);
}


Step three, sort and select - at that point that can go inside GetByPage:

public IEnumerable GetByPage(int page, int pageSize, IEnumerable categories)
{
    var items = FilterForSpecifiedPage(page, pageSize, categories);
    return (from blog in items.Include(item => blog.Category)
                              .Include(item => blog.User)
            orderby blog.CreationDateStamp ascending
            select blog)
           .ToList();
}


Now, this is strictly equivalent and improves nothing but readability, by un-nesting the queries.

I believe the .Include calls are redundant anywhere other than in the final step, in GetByPage - by including them so deep in the query you're carrying more data than you actually need along the way; I'd try removing them in the two private methods, and see if I get the same results.

Notice I'm returning an IEnumerable and not a List - the client code shouldn't be adding or removing items from this result set, and so doesn't need a List. Rather, it merely needs to enumerate the results, so IEnumerable is enough for its needs. And it's an interface, so you're not tying your hands with a specific implementation either.

Your pagination seems rather inefficient, you're sorting back and forth and back again.

Instead of sorting backwards and Takeing, you could look into what Skip can do for you - I believe it could boil down to something like this (untested):

return (from blog in dbSet.Include(item => item.Category)
                          .Include(item => item.User)
        where categories.Contains(blog.CategoryId)
        order by blog.CreationDate)
       .Skip((page - 1) * pageSize)
       .Take(pageSize)
       .ToList();


...which basically means, scratch pretty much everything I said above about extracting to private methods :)

Code Snippets

return (from b1 in ((from b2 in ((from b3 in dbSet.Include(b3 => b3.Category).Include(b3 => b3.User)
                                  orderby b3.CreationDateStamp ascending
                                  where categories.Contains(b3.CategoryId)
                                  select b3).Take<Blog>(page * pagecount)).Include(b2 => b2.Category).Include(b3 => b3.User) // step one
                     orderby b2.CreationDateStamp descending
                     select b2).Take<Blog>(pagecount)).Include(b1 => b1.Category).Include(b3 => b3.User) // step two
        orderby b1.CreationDateStamp ascending
        select b1).ToList<Blog>(); // step tree
private IEnumerable<Blog> FetchItemsUpToSpecifiedPage(int page, int pageSize, IEnumerable<int> categories)
{
    return (from blog in dbSet.Include(item => blog.Category)
                              .Include(item => blog.User)
            orderby blog.CreationDateStamp ascending
            where categories.Contains(blog.CategoryId)
            select blog)
           .Take(page * pageSize);
}
private IEnumerable<Blog> FilterForSpecifiedPage(int page, int pageSize, IEnumerable<int> categories)
{
    var items = FetchItemsUpToSpecifiedPage(page, pageSize, categories);
    return (from blog in items.Include(item => blog.Category)
                              .Include(item => blog.User)
            orderby blog.CreationDateStamp descending
            select blog)
           .Take(pageSize);
}
public IEnumerable<Blog> GetByPage(int page, int pageSize, IEnumerable<int> categories)
{
    var items = FilterForSpecifiedPage(page, pageSize, categories);
    return (from blog in items.Include(item => blog.Category)
                              .Include(item => blog.User)
            orderby blog.CreationDateStamp ascending
            select blog)
           .ToList();
}
return (from blog in dbSet.Include(item => item.Category)
                          .Include(item => item.User)
        where categories.Contains(blog.CategoryId)
        order by blog.CreationDate)
       .Skip((page - 1) * pageSize)
       .Take(pageSize)
       .ToList();

Context

StackExchange Code Review Q#120420, answer score: 4

Revisions (0)

No revisions yet.