I've struggled with this for quite some time. Today I finally wrote the following code.

The ViewModel contains an int property which later tells the view how many pages the data has been split into.

The controller splits the data by taking a specified amount of rows and, in the event of paging, splits by pageNumber * recordsPerPage

Take a look:

The ViewModel

public class ThreadPostsViewModel
    public Thread Thread { get; set; }
    public List<Post> Posts { get; set; }
    public int Pages { get; set; }

The Controller

private int PostsPerPage = 10;

public ActionResult Thread(int id, int page = 1)
    using (OrtundEntities Db = new OrtundEntities())
        // get the thread and its parent data (parent for breadcrumbs)
        var Thread = Db.Threads.Include(t => t.Title).FirstOrDefault(x => x.Id == id);

        // create a list for the Posts
        List<Post> Posts = new List<Post>();

        // select based on paging
        if (page == 1)
            // no paging has happened, get the first set of records
            Posts = Db.Posts.Include(x => x.User).Where(x => x.ThreadId == id).OrderByDescending(x => x.Date).Take(PostsPerPage).ToList();
            // we're on a new page. Skip however many rows we've already seen
            Posts = Db.Posts.Include(x => x.User).Where( x=> x.ThreadId == id).OrderByDescending(x => x.Date).Take(PostsPerPage).Skip(PostsPerPage * page).ToList();

        // create and return the view model
        ThreadPostsViewModel Model = new ThreadPostsViewModel
            Thread = Thread,
            Posts = Posts,
            Pages = Posts.Count / PostsPerPage

        return View(Model);

The View

@model Ortund.Models.ThreadPostsViewModel
<div class="paging">
    @for (int i = 1; i < Model.Pages; i++)
        string Url = String.Format("/View/Thread/{0}?page={1}", Model.Thread.Id, i);

        <a href="@Url">@i</a>
<div class="posts-list">
    @foreach (var Post in Model.Posts)
        <div class="post" id="@Post.Id">


In this code, assuming 300 posts are selected from the database and 10 posts are specified per page, then there should be 30 pages.

  • \$\begingroup\$ I'm voting for closing becase this code is broken and doesn't do what it's supposed to do. \$\endgroup\$ – t3chb0t Sep 5 '16 at 4:34
  • \$\begingroup\$ That's a lousy reason to just throw the question away... \$\endgroup\$ – Ortund Sep 5 '16 at 10:33
  • 2
    \$\begingroup\$ If you read the rules you'd know that only working code is subject to review. \$\endgroup\$ – t3chb0t Sep 5 '16 at 10:34
  • 3
    \$\begingroup\$ On Code Review it is our policy to review code that fully works as intended. Please read How to Ask and help center. \$\endgroup\$ – janos Aug 22 '17 at 7:32

Your naming is incosistent. For some variables you use var Thread and for others the full name ThreadPostsViewModel Model. Besides we name local variables in camelCase.

You can greatly shorten this query by not repeating yourself and create the result model at once.

Did you test it? Because this contains several bugs and doesn't do what you think it does.

If you Take(PostsPerPage) and then .Skip(PostsPerPage * page) then you'll have nothing left.

This should be the other way around and if for the Skip you write PostsPerPage * (page - 1) then for the page 1 you'll skip 0 records so you don't need two queries.

Also this Posts.Count / PostsPerPage will always return 1 - it should be Db.Pages.Count / PostsPerPage.

var result = new ThreadPostsViewModel
    Thread = Db.Threads
        .Include(t => t.Title)
        .FirstOrDefault(x => x.Id == id),
    Posts = Db.Posts
        .Include(x => x.User)
        .Where( x=> x.ThreadId == id)
        .OrderByDescending(x => x.Date)
        .Skip(PostsPerPage * (page - 1))
    Pages = Db.Pages.Count / PostsPerPage
| improve this answer | |
  • \$\begingroup\$ Not to argue about the naming conventions but... The PascalCasing convention, used for all identifiers except parameter names, capitalizes the first character of each word (including acronyms over two letters in length). This from msdn.microsoft.com/en-us/library/ms229043(v=vs.110).aspx \$\endgroup\$ – Ortund Sep 5 '16 at 9:19
  • \$\begingroup\$ Also Db.Pages.Count isn't actually a thing in my data model. I'm deriving the amount of pages from the total number of posts / PostsPerPageso if I have 420 posts, and 10 PostsPerPage, then Pages should work out to 420 / 10 = 42 although it just occurred to me that I should add a page in cases where this division yields a remainder (so 426 posts would result in 43 page where page 43 has only 6 posts on it) \$\endgroup\$ – Ortund Sep 5 '16 at 9:57

Not the answer you're looking for? Browse other questions tagged or ask your own question.