Page 1 of 1

Simple pagination script

Posted: Sat Jan 03, 2009 7:43 pm
by MichaelR
Okay, so I've written my own pagination script because I feel the ones that can be found on the internet are too complex. I much prefer to keep things as simple and concise as possible. However, I'm aware that when things are simplified, such things can lose some of their functionality. That's why I'd like you to critique this code and let me know if anything is missing and/or problematic.

Code: Select all

 
 if (isset($_POST["location"]))
  $location = mysql_real_escape_string($_POST["location"]);
 
 elseif (isset($_GET["location"]))
  $location = mysql_real_escape_string($_GET["location"]);
 
 $total = mysql_num_rows(mysql_query("SELECT `id` FROM `users` WHERE `location` = '".$location."'"));
 
 $limit = 10;
 
 $pages = ($total >= 1) ? ceil($total / $limit) : 1;
 
 $page  = ($_GET["page"] >= 1 && $_GET["page"] <= $pages) ? $_GET["page"] : 1;
 
 $lower = $page * $limit - $limit;
 
 $query = mysql_query("SELECT `username` FROM `users` WHERE `location` = '".$location."' LIMIT ".$lower.", ".$limit);
 
 while ($result = mysql_fetch_array($query))
  echo $result["username"]."<br />";
 
 for ($i = 1; $i <= $pages; $i++) {
 
  if ($i == $page)
   echo $i;
 
  elseif ($i <= 3 || $i <= 7 && $page <= 5 || $i == $page - 1 || $i == $page + 1 || $i > $pages - 7 && $page > $pages - 5 || $i > $pages - 3 || $pages <= 11)
   echo "<a href=\"".$link.".php&page=".$page."&location=".$location."\">".$i."</a>";
 
  elseif ($i == 4 || $i == $pages - 3) 
   echo "&hellip;";
 
 }
 

Re: Simple pagination script

Posted: Sat Jan 03, 2009 8:38 pm
by califdon
The first part looks correct, to me. I don't understand what you're trying to do in that for loop at the bottom. I would recommend that you validate the $_GET['page'] value, though, before using it.

Re: Simple pagination script

Posted: Sat Jan 03, 2009 9:33 pm
by John Cartwright

Code: Select all

$total = mysql_num_rows(mysql_query("SELECT `id` FROM `users` WHERE `location` = '".$location."'"));
You are unnecesarily querying your entire dataset. If all you need the total number of rows, use COUNT(id) instead.

Code: Select all

$result = mysql_query("SELECT COUNT(`id`) AS `total` FROM `users` WHERE `location` = '".$location."'");
$row = mysql_fetch_assoc($result);
$total = $row['total'];

Re: Simple pagination script

Posted: Sun Jan 04, 2009 6:30 am
by MichaelR
califdon wrote:The first part looks correct, to me. I don't understand what you're trying to do in that for loop at the bottom. I would recommend that you validate the $_GET['page'] value, though, before using it.
I'll give you a few examples of the for loop in action.

Code: Select all

 
 ($pages == 11)
 
 1 2 3 4 5 6 7 8 9 10 11
 
 ($page <= 5 && $pages == 13)
 
 1 2 3 4 5 6 7 ... 11 12 13
 
 ($page > $pages - 5 && $pages == 13)
 
 1 2 3 ... 7 8 9 10 11 12 13
 
 $page == 7 && pages == 13)
 
 1 2 3 ... 6 7 8 ... 11 12 13
 
And I didn't think I'd have to worry about validating the $_GET["page"] variable. Only if $_GET["page"] is greater than or equal to 1, and less than or equal to $pages, will it be used; else the $page equals 1. Anything other than a valid numeric would fail the if statement, and so the page defaults to 1.
Jcart wrote:You are unnecesarily querying your entire dataset. If all you need the total number of rows, use COUNT(id) instead.
Thank you. And does it matter if I use mysql_fetch_array over mysql_fetch_assoc? Or should I use the latter?

Re: Simple pagination script

Posted: Sun Jan 04, 2009 8:39 am
by VladSun
Jcart wrote:You are unnecessarily querying your entire dataset. If all you need the total number of rows, use COUNT(id) instead.

Code: Select all

$result = mysql_query("SELECT COUNT(`id`) AS `total` FROM `users` WHERE `location` = '".$location."'");
$row = mysql_fetch_assoc($result);
$total = $row['total'];
There is some ready-to-use functionality in MySQL for solving this problem:
[sql]SELECT SQL_CALC_FOUND_ROWS id, username, email FROM users WHERE username < 'B' LIMIT 10;SELECT FOUND_ROWS();[/sql]

No coma after SQL_CALC_FOUND_ROWS .

Re: Simple pagination script

Posted: Sun Jan 04, 2009 8:50 am
by VladSun
MichaelR wrote:And I didn't think I'd have to worry about validating the $_GET["page"] variable. Only if $_GET["page"] is greater than or equal to 1, and less than or equal to $pages, will it be used; else the $page equals 1. Anything other than a valid numeric would fail the if statement, and so the page defaults to 1.

Code: Select all

$_GET['page'] = "2;<script>alert('XSS')</script>"
You don't have an echo $page in your current code, but it's not for sure you will not use it somewhere else. So - always validate, sanitize and filter the user input.
MichaelR wrote:Thank you. And does it matter if I use mysql_fetch_array over mysql_fetch_assoc? Or should I use the latter?
mysql_fetch_array will return a two times longer array than mysql_fetch_assoc - it will contain both associative array and a zero based indexed array. So, I'd use mysql_fetch_assoc, or mysql_fetch_object, or mysql_fetch_row, but not mysql_fetch_array.

Re: Simple pagination script

Posted: Sun Jan 04, 2009 8:59 am
by MichaelR
VladSun wrote:You don't have an echo $page in your current code, but it's not for sure you will not use it somewhere else. So - always validate, sanitize and filter the user input.
Thank you, I will remember to do so. Better safe than sorry, I guess.
mysql_fetch_array will return a two times longer array than mysql_fetch_assoc - it will contain both associative array and a zero based indexed array. So, I'd use mysql_fetch_assoc, or mysql_fetch_object, or mysql_fetch_row, but not mysql_fetch_array.
Would that be a blanket rule? Or just for something like this pagination script? So, say I wished to select a user's data:

Code: Select all

 
$sql = "SELECT * FROM `users` WHERE `id` = ".$_SESSION["id"]." LIMIT 1";
 
$resultone = mysql_fetch_array(mysql_query($sql));
$resulttwo = mysql_fetch_assoc(mysql_query($sql));
 
Would assoc be of better use here, as well?

Re: Simple pagination script

Posted: Sun Jan 04, 2009 9:02 am
by VladSun
MichaelR wrote:

Code: Select all

 
$sql = "SELECT * FROM `users` WHERE `id` = ".$_SESSION["id"]." LIMIT 1";
 
$resultone = mysql_fetch_array(mysql_query($sql));
$resulttwo = mysql_fetch_assoc(mysql_query($sql));
 
Would assoc be of better use here, as well?
Add

Code: Select all

print_r($resultone);
print_r($resulttwo);
and you will see what's the difference.
:)

Re: Simple pagination script

Posted: Sun Jan 04, 2009 9:15 am
by MichaelR
VladSun wrote:

Code: Select all

 
print_r($resultone);
print_r($resulttwo);
and you will see what's the difference.
Ah, yes. I see. I'll never use something like $result[0] in these situations, so it seems superfluous to use fetch_array. Thanks again.