views:

75

answers:

3

I'm working on a pagination algorithm in PHP. I can guess that it needs room for improvement, so I'd like some thoughts on how to improve it, be it cleaning up the code itself, from a UI/UX standpoint, or anything else you can think of.

The algorithm should output pagination that looks like this:

1 2 3 ... 6 7 8 ... 97 98 99

or this:

1 2 3 4 5 ... 6 7 8 9 10

or this:

1 2 3

Here's my code:

<?php

if(!is_null($_GET['page'])) {
  $currentPage = $_GET['page'];
} else {
  $currentPage = 1;
}

if($pages != null) {
  echo 'Page: ';
}

// Less than 10 pages
if($pages <= 10) {
  for($page = 1; $page <= $pages; $page++) {
    echo '<a href="?page=' . $page . '">' . $page . '</a> ';
  }

// Greater than 10 pages, and we're somewhere in the middle of them
} elseif(($pages > 10) && ($currentPage > 4) && ($currentPage < $pages - 3)) {
  for($page = 1; $page <= 3; $page++) {
    echo '<a href="?page=' . $page . '">' . $page . '</a> ';
  }
  echo '... ';
  for($page = $currentPage - 1; $page <= $currentPage + 1; $page++) {
    echo '<a href="?page=' . $page . '">' . $page . '</a> ';
  }
  echo '... ';
  for($page = $pages - 2; $page <= $pages; $page++) {
    echo '<a href="?page=' . $page . '">' . $page . '</a> ';
  }

// Greater than 10 pages, and we're towards the end of the pages
} else {
  for($page = 1; $page <= 5; $page++) {
    echo '<a href="?page=' . $page . '">' . $page . '</a> ';
  }
  echo '... ';
  for($page = $pages - 5; $page <= $pages; $page++) {
    echo '<a href="?page=' . $page . '">' . $page . '</a> ';
  }
}
A: 

I'm not sure if my code is any better than yours, but here is how I solved a similar problem.

It takes a parameter for the amount of pages to generate and creates a div with the class pages containing anchors, the current page has a class of current. This solution seems a little cleaner because there is less repetition.

function generate_pages($total,$current)
{ //Will generate pages and link to ?page=x when passed total pages to output
    if($total > 1)
    {
        $total=intval($total);

        $output='<div class="pages">';
        $current_page= (false == isset($current)) ? 1 : $current;
        for($page=1;$page<$total+1;$page++)
        {
            $lower=$current_page-3;
            $upper=$current_page+3;
            $special = ($page==$current_page) ? " class=\"current\"" : "";
            if(($page > $lower && $page < $upper) || $page < 2 || $page > ($total-1))
            {
                if($last_done_page+1 != $page) $output.= '... ';
                $output.='<a'.$special.' href="?page='.$page.'">'.$page.'</a>';
                $last_done_page=$page;
            }
        }
        $output.='</div>';
        return $output;
    }
}
Sam152
@Sam152: you need to extract the global inside your function (`$_GET['page']`) because it's not the view layer that should handle that kind of logic. Make it a parameter of the function instead, otherwise you've locked your app. into not being able to use the variable `$_GET['page']` for anything else, not very portable.
chelmertz
Updated the code a little.
Sam152
A: 

here is a similar post: http://www.strangerstudios.com/sandbox/pagination/diggstyle.php

I think this would serve better as a comment than an answer, unless you can bring something there over to the discussion here (other than a link).
Josh Smith
+1  A: 

another good function of pagination in this article

http://www.frobie.com/pagination-with-php/2008-03-18/

Haim Evgi