Paginating blog posts from SQL server using nested LINQ statements

Question

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<Blog>
{
 public List<Blog> GetByPage(int page, List<int> 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<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
 }
}

Here is my way of working:

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

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

3. Finally, sort it again (ascending), and call ToList<Blog>() to materialize the results.

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

Note two: the variable dbSet comes from a generic repository and is declared as follows:

public class GenericRepo<TEntity> : IGenericRepo<TEntity> where TEntity : class
{
 internal DbSet<TEntity> dbSet;
 // other basic linq statements for get basic stuff like GetAll, Delete, Insert, SaveChanges and Update
}

The code works, but I wondering if I can improve its performance and readability. In other words, can I combine some steps and can I reduce the code to make it shorter or simpler?

Answer 1

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<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

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<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);
}

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<T> 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<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);
}

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

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();
}

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<Blog> and not a List<Blog> - the client code shouldn't be adding or removing items from this result set, and so doesn't need a List<Blog>. Rather, it merely needs to enumerate the results, so IEnumerable<Blog> 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 :)

• H. Pauwelyn: The reason why I used b3 is because b is the first leter and the number indicate is the step. Thanks a lot to give better code. :)