patterncsharpMinor
Paginating blog posts from SQL server using nested LINQ statements
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
Each Blog has a Category. I'll also that the user can see one, more, or all categories (variable named
Here is my way of working:
-
I take the first items calculated by the formula
-
Form that items I sort it in descending order on the same property, and take the value of
-
Finally, sort it again (ascending), and call
Note one: I'll always include the properties
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 `ApSolution
Your linq in linq in linq statement is written as a one-liner, with a single semicolon:
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:
Notice the following:
Step two, filter for specified page - again, in its own method:
Step three, sort and select - at that point that can go inside
Now, this is strictly equivalent and improves nothing but readability, by un-nesting the queries.
I believe the
Notice I'm returning an
Your pagination seems rather inefficient, you're sorting back and forth and back again.
Instead of sorting backwards and
...which basically means, scratch pretty much everything I said above about extracting to private methods :)
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 treeIn 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:
pagecountis renamed to a more accuratepageSize; "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.
blogmakes for easier reading thanb3.
- The compiler infers the generic type argument of
Takefrom thesourceparameter 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 treeprivate 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.