views:

28

answers:

1

Assuming that the mysqli object is already instantiatied (and connected) with the global variable $mysql, here is the code I am trying to work with.

class Listing {
private $mysql;
function getListingInfo($l_id = "", $category = "", $subcategory = "", $username = "", $status = "active") {
        $condition = "`status` = '$status'";
        if (!empty($l_id)) $condition .= "AND `L_ID` = '$l_id'";
        if (!empty($category)) $condition .= "AND `category` = '$category'";
        if (!empty($subcategory)) $condition .= "AND `subcategory` = '$subcategory'";
        if (!empty($username)) $condition .= "AND `username` = '$username'";
        $result = $this->mysql->query("SELECT * FROM listing WHERE $condition") or die('Error fetching values');
        $this->listing = $result->fetch_array() or die('could not create object');
        foreach ($this->listing as $key => $value) :
            $info[$key] = stripslashes(html_entity_decode($value));
        endforeach;
        return $info;
    }
}

there are several hundred listings in the db and when I call $result->fetch_array() it places in an array the first row in the db. however when I try to call the object, I can't seem to access more than the first row. for instance: $listing_row = new Listing; while ($listing = $listing_row->getListingInfo()) { echo $listing[0]; }

this outputs an infinite loop of the same row in the db. Why does it not advance to the next row? if I move the code:

$this->listing = $result->fetch_array() or die('could not create object');
        foreach ($this->listing as $key => $value) :
            $info[$key] = stripslashes(html_entity_decode($value));
        endforeach;

if I move this outside the class, it works exactly as expected outputting a row at a time while looping through the while statement. Is there a way to write this so that I can keep the fetch_array() call in the class and still loop through the records?

A: 

Your object is fundamentally flawed - it's re-running the query every time you call the getListingInfo() method. As well, mysql_fetch_array() does not fetch the entire result set, it only fetches the next row, so your method boils down to:

  1. run query
  2. fetch first row
  3. process first row
  4. return first row

Each call to the object creates a new query, a new result set, and therefore will never be able to fetch the 2nd, 3rd, etc... rows.

Unless your data set is "huge" (ie: bigger than you want to/can set the PHP memory_limit), there's no reason to NOT fetch the entire set and process it all in one go, as shown in Jacob's answer above.

as a side note, the use of stripslashes makes me wonder if your PHP installation has magic_quotes_gpc enabled. This functionality has been long deprecrated and will be removed from PHP whenever v6.0 comes out. If your code runs as is on such an installation, it may trash legitimate escaping in the data. As well, it's generally a poor idea to store encoded/escaped data in the database. The DB should contain a "virgin" copy of the data, and you then process it (escape, quote, etc...) as needed at the point you need the processed version.

Marc B
@mark :Thanks for the explanation. I'm very new to OOP. I do have a question regarding security as a follow up. I don't have magic_quotes_gpc enabled but I have been using addslashes() to everything that is sent through a form before I add it to the database. Is there a better way to secure data? How else should I filter the data that someone sends via form input so that I can safely enter it into my database?
Jonathan
The best way to write 'secure' queries is to use PDO (http://php.net/PDO) with prepared statements. Those will handle all the heavy lifting of quoting/escaping data. Otherwise it's best to use mysql_real_escape_string() rather than addslashes(). The mysql function uses mysql's own quoting mechanisms. addslashes is basically just a dumb string replacement function and could potentially be fooled by unicode/unconventional characters.
Marc B