This code is efficient?

[Azoulai]

I have a code that division a data from MySql table to pages,
My question is, if this code is an efficient (upload fast)?
If not tell me please how improve this code to make him more efficient.

(I don't write this code)

Sorry about my english🙁

<table width="100%">

<tr><td>
<?php

include ("config.php");

mysql_pconnect( "localhost", "$username", "$password")
               or die( "Unable to connect to SQL server");
mysql_select_db( "$db_name") or die( "Unable to select database");

$query = mysql_query("SELECT count(*) as total FROM `members`") or die(mysql_error());
$row = mysql_fetch_array($query,MYSQL_ASSOC);
$numrows = $row['total'];

if (empty($offset)) {
    $offset=0;
}

$query = "SELECT user, pass, email, id FROM members order by id DESC limit $offset,$limit";
$result = mysql_query ($query)
    or die ("query 2 failed");

while ($row = mysql_fetch_row ($result))
{
for ($i = 0; $i < mysql_num_fields ($result); $i++)
    {
    if ($i > 0)
        print ("
");    
    if ($i == 0){
        print "<b>Name: </b>";    
    }
    else if ($i == 1){
        print "<b>Date: </b>";
    }
    else if ($i == 2){
        print "<b>Email: </b>";
    }
    else{
        print "<b>Comment:</b>
";
    }

    print ($row[$i]);

    }

print "

<center><hr></center>";
print ("<P>");
}
if ($offset >= 3) {
$prevoffset = $offset - $limit;
print "<a href=\"?offset=$prevoffset\">PREV</a> \n";
}
$pages=intval($numrows/$limit);
if ($pages < ($numrows/$limit)){
$pages=($pages + 1);
}
for ($i = 1; $i <= $pages; $i++) {
$newoffset = $limit*($i-1);
if ($newoffset == $offset) {
print "$i \n";
} else {
print "<a href=\"?offset=$newoffset\">$i</a> \n";
}
}

//show next if not last
if (! ( ($offset/$limit) == ($pages - 1) ) && ($pages != 1) ) {
$newoffset = $offset+$limit;
print "<a href=\"?offset=$newoffset\">NEXT</a><p>\n";
}
?>
</td></tr>
</table>
</body>
</html>

[IceD]

Code seems to be more-less OK (but damn inaccurate). The only thing, I can't stand looking at is mysql_pconnect (pconnects are evil).

[Azoulai]

and other thing, I searched easy way to print the data,
you see, in the code have a lot of Conditionals (IF and WHILE and FOR) I can change it and make more easy for me to print the data?

[Weedpacket]

Originally posted by Azoulai
and other thing, I searched easy way to print the data,
you see, in the code have a lot of Conditionals (IF and WHILE and FOR) I can change it and make more easy for me to print the data?

Um, as far as I can see you're displaying the "pass" field under Date: and the "id" field under Comment:. Is this what you're wanting to do? If not, then you'll appreciate the fact that mysql_fetch_array() can give you an array with fields indexed by field name. Because your whole for{} loop is equivalent to

print('<b>Name: </b>'.$row['user']);
print(' <b>Date: </b>'.$row['pass']);
print(' <b>Email: </b>'.$row['email']);
print(' <b>Comment: </b>'.$row['id']);

Another thing: $offset on my system will always be empty, because register_globals is turned off (by default). The offset will be in $_GET['offset'] instead. And because it's coming from the outside world, it can't be trusted to have a sensible value - so you really must check to make sure that really it is a suitable number before sticking it into your SQL query.

[Azoulai]

I write this in While loop:

print('<b>Name: </b>'.$row['user']);
print(' <b>Date: </b>'.$row['pass']);
print(' <b>Email: </b>'.$row['email']);
print(' <b>Comment: </b>'.$row['id']);

instead For loop?

And now I should add this:

if($page>2000000000 || $page<1)
$page=1;

and add too:

$page = (int) $_GET['page'];

This what you mean?