Trying to convert code to prepared statements

[vinsb]

Hello,

as I am new in php and absolutely new to mysqli_* prepared statements I need a little help. I'm trying to convert some code but I have troubles.

$qry = $con->prepare("SELECT * FROM images order by ? DESC");
            $qry->bind_param('i', $id);
            $qry->execute();
            $result = $qry->get_result();
            $result->fetch_array();

// I don't realy know what to do after this line

            $line = mysqli_fetch_array($result, MYSQL_BOTH);
            if (!$line) echo '';
            $previd = -1;
            $currid = $line[0];
            if (isset($_GET['id'])) {
                do {
                    $currid = $line[0];
                    if ($currid == $_GET['id']) break;
                    $previd = $currid;
                    $line = mysqli_fetch_array($result, MYSQL_BOTH);
                } while ($line);
            }

            if ($line) {
                echo "<div id=\"picture\">";
                echo "<img style=\"width:100%;margin:0 auto;\" src=\"upload/".$line['name']."\" /></a><br />";
                echo "<div id=\"caption\">".$line['caption']."</div><br />";
            }
            else echo "Empty!\n";

            if ($previd > -1) echo '<a href="pics.php?id='.$previd.'" class="prev_pic"><span>prev</span></a>';
            echo str_repeat(' ', 5);

            $line = mysqli_fetch_array($result, MYSQL_BOTH);

            $query = "select * from images order by RAND() LIMIT 1";
            $result = mysqli_query($con, $query) or die("Query failed: " . mysqli_errno($con));
            while ($row = mysqli_fetch_array($result, MYSQL_BOTH)){
                echo '<a href="pics.php?id='.$row['id'].'"class="random">rand</a>';
            }
            echo str_repeat(' ', 5);
            if ($line) echo '<a href="pics.php?id='.$line[0].'" class="next_pic"><span>next</span> </a><br /><br />';

So this also work but doesn't show me First and Last record of DB. Also I know that now is mixed style of mysqli_* but thats the problem.

[Augury]

You could solve half your problem by adding a stub row to your DB.

[Augury]

$line = mysqli_fetch_array($result, MYSQL_BOTH);

 

This will move the array pointer and you will have to reset that array pointer or devise way to use the pointer as it is.

[vinsb]

You could solve half your problem by adding a stub row to your DB.

 

 

$line = mysqli_fetch_array($result, MYSQL_BOTH);

 

This will move the array pointer and you will have to reset that array pointer or devise way to use the pointer as it is.

Thank's for answer!

I'm sorry but I don't understand you. Can you clarify your answer a little?

Edited March 7, 2014 by vinsb

[Ch0cu3r]

Can you explain what it is you are trying to do. I do not understand the logic in your code.

[vinsb]

Can you explain what it is you are trying to do. I do not understand the logic in your code.

I trying to rewrite my code with prepared statements to preventing SQL injection.

This code take image from mysql and display on page. There is also buttons for 'Prev' and 'Next' image. As I said I'm new in php and may be my code is a bit mess but at least is working so far. 

First I made this

$qry = $con->prepare("SELECT * FROM images order by ? DESC");
            $qry->bind_param('i', $id);
            $qry->execute();
            $result = $qry->get_result();
            $result->fetch_array();

Before it was like this

            $query = "select * from images order by id DESC";
            $result = mysqli_query($con, $query) or die("Query failed: " . mysqli_errno($con));
 

But I can't understand how to rewrite other part of code.

$line = mysqli_fetch_array($result, MYSQL_BOTH);
            if (!$line) echo '';
            $previd = -1;
            $currid = $line[0];
            if (isset($_GET['id'])) {
                do {
                    $currid = $line[0];
                    if ($currid == $_GET['id']) break;
                    $previd = $currid;
                    $line = mysqli_fetch_array($result, MYSQL_BOTH);
                } while ($line);
            }

            if ($line) {
                echo "<div id=\"picture\">";
                echo "<img style=\"width:100%;margin:0 auto;\" src=\"upload/".$line['name']."\" /></a><br />";
                echo "<div id=\"caption\">".$line['caption']."</div><br />";
            }
            else echo "Empty!\n";

            if ($previd > -1) echo '<a href="pics.php?id='.$previd.'" class="prev_pic"><span>prev</span></a>';
            echo str_repeat(' ', 5);

            $line = mysqli_fetch_array($result, MYSQL_BOTH);

            $query = "select * from images order by RAND() LIMIT 1";
            $result = mysqli_query($con, $query) or die("Query failed: " . mysqli_errno($con));
            while ($row = mysqli_fetch_array($result, MYSQL_BOTH)){
                echo '<a href="pics.php?id='.$row['id'].'"class="random">rand</a>';
            }
            echo str_repeat(' ', 5);
            if ($line) echo '<a href="pics.php?id='.$line[0].'" class="next_pic"><span>next</span> </a><br /><br />';

Edited March 7, 2014 by vinsb

[mac_gyver]

since you cannot specify a column name using a place-holder ? in a prepared query, your first query won't work anyway.

 

prepared queries, for the purpose of preventing sql injection, use place-holders and bind data values into the query for numbers and strings.

 

the order by id part in your first query is 'static' there's nothing in this that uses an external data value.

 

if you had a where clause in your original query, something like WHERE id = $id, you could convert this to change the $id into a place-holder ? and use a prepared query.

 

---

 

the reason Ch0cu3r asked about what your logic is trying to do (he is not asking if you are trying to convert to it to use prepared queries, we already know that) is because your code is difficult to follow, i.e. we cannot tell what it is supposed to be doing at all because it is a jumble of queries mixed in with html markup, using numeral array offsets that don't tell us the meaning of the data...

 

so, rather than posting your code, describe the end result you are trying to produce for the images and links.

[vinsb]

Simply select image from database. Display that image on page and buttons 'Prev' 'next' and 'Rand' navigate to next and prev image from database.

Here is the image of end result.

I just don't know how to make it better yet.

Edited March 7, 2014 by vinsb

[mac_gyver]

your problem has two parts -

 

1) accept a page request and retrieve and display the correct image.

 

2) produce previous, next, and random links that when submitted tell part #1 what it should do.

 

to keep the number of database queries to a minimum, your previous, next, and random links should only tell the retrieval code what to do, rather than to preselect the id's to use. the only time you run any queries should be in step #1.

 

there are two ways of handling the previous/next operation. 1) you can use pagination with a page size of one (this uses a LIMIT x,y in the query to select the correct row(s) from your table), or 2) you can use 'rowination' where you operate by finding the next lower or higher row's id. pagination is more general purpose, since you can change it to display any number of images simply by changing the page size value the code uses.  there's a good pagination tutorial here - http://forums.phpfreaks.com/page/tutorials/_/basic-pagination

 

lastly, as to converting your code to use prepared queries, after you get the logic doing what you want, you need to separate the code running the queries (the business logic) from the presentation code containing the html markup. by having this separation, the code running the queries will be grouped together and converting it will only involve making changes in the business logic. you won't have to touch the presentation code.

[mac_gyver]

handling the random link using the pagination method would/could work as follows -

 

1) pass the 'current' page number in the link (so that you can exclude the current image/page from the random selection), and pass a second parameter in the link that indicates you want to get a random image/page number.)

 

2) after you have gotten the total number of images/pages in the pagination code, use range() to create an array of all the possible images/pages, then remove the 'current' image/page number from that array. randomize the array and get the first entry. use this value as input to the pagination logic for the image/page number to retrieve.

 

a possible enhancement to this would be to store the 'current' page number that comes with the link as an array in a session variable. then you can remove all the previously seen images/pages from the result of the range() function so that you don't repeat any images/pages until all of them have been viewed.

[vinsb]

Thank you for your explanation. I know that my code is ... mess. I will try to make it right and better. 

 

p.s.

The link that you gave me isn't work. It's empty page.

[mac_gyver]

p.s.

The link that you gave me isn't work. It's empty page.

 

apparently the tutorials are only visible to members above some level. here is the php code that is found in that tutorial -

<?php
// database connection info
$conn = mysql_connect('localhost','dbusername','dbpass') or trigger_error("SQL", E_USER_ERROR);
$db = mysql_select_db('dbname',$conn) or trigger_error("SQL", E_USER_ERROR);

// find out how many rows are in the table
$sql = "SELECT COUNT(*) FROM numbers";
$result = mysql_query($sql, $conn) or trigger_error("SQL", E_USER_ERROR);
$r = mysql_fetch_row($result);
$numrows = $r[0];

// number of rows to show per page
$rowsperpage = 10;
// find out total pages
$totalpages = ceil($numrows / $rowsperpage);

// get the current page or set a default
if (isset($_GET['currentpage']) && is_numeric($_GET['currentpage'])) {
    // cast var as int
    $currentpage = (int) $_GET['currentpage'];
} else {
    // default page num
    $currentpage = 1;
} // end if

// if current page is greater than total pages...
if ($currentpage > $totalpages) {
// set current page to last page
    $currentpage = $totalpages;
} // end if
// if current page is less than first page...
if ($currentpage < 1) {
// set current page to first page
    $currentpage = 1;
} // end if

// the offset of the list, based on current page
$offset = ($currentpage - 1) * $rowsperpage;

// get the info from the db
$sql = "SELECT id, number FROM numbers LIMIT $offset, $rowsperpage";
$result = mysql_query($sql, $conn) or trigger_error("SQL", E_USER_ERROR);

// while there are rows to be fetched...
while ($list = mysql_fetch_assoc($result)) {
    // echo data
    echo $list['id'] . " : " . $list['number'] . "<br />";
} // end while

/****** build the pagination links ******/
// range of num links to show
$range = 3;

// if not on page 1, don't show back links
if ($currentpage > 1) {
    // show << link to go back to page 1
    echo " <a href='{$_SERVER['PHP_SELF']}?currentpage=1'><<</a> ";
    // get previous page num
    $prevpage = $currentpage - 1;
    // show < link to go back to 1 page
    echo " <a href='{$_SERVER['PHP_SELF']}?currentpage=$prevpage'><</a> ";
} // end if

// loop to show links to range of pages around current page
for ($x = ($currentpage - $range); $x < (($currentpage + $range) + 1); $x++) {
    // if it's a valid page number...
    if (($x > 0) && ($x <= $totalpages)) {
        // if we're on current page...
        if ($x == $currentpage) {
            // 'highlight' it but don't make a link
            echo " [<b>$x</b>] ";
        // if not current page...
        } else {
            // make it a link
            echo " <a href='{$_SERVER['PHP_SELF']}?currentpage=$x'>$x</a> ";
        } // end else
    } // end if
} // end for

// if not on last page, show forward and last page links
if ($currentpage != $totalpages) {
    // get next page
    $nextpage = $currentpage + 1;
    // echo forward link for next page
    echo " <a href='{$_SERVER['PHP_SELF']}?currentpage=$nextpage'>></a> ";
    // echo forward link for lastpage
    echo " <a href='{$_SERVER['PHP_SELF']}?currentpage=$totalpages'>>></a> ";
} // end if
/****** end build pagination links ******/
?>

as to using prepared queries for this, the queries in this code don't have any direct external values being put into them and won't benefit by using a prepared query.

Edited March 7, 2014 by mac_gyver

[vinsb]

Thank you a lot!