BindParam() not working properly

Question

In my DB class I have a method query() which takes the SQL statement and the parameters as arguments and runs them in the database.

    // the parameters are the sql statement and the values, returns the the current object
    // the result set object can be accessed by using the results() method

    public function query($sql, $params = array()) {

        $this->_error = false;  // initially the error is set to false

        if($this->_query = $this->_pdo->prepare($sql)) {

            // if the parameters are set, bind it with the statement
            if(count($params)) {
                foreach($params as $param => $value) {
                    $this->_query->bindParam($param, $value);
                }
            }

            // if the query is successfully executed save the resultset object to the $_results property
            // and store the total row count in $_count
            if($this->_query->execute()) {
                $this->_results = $this->_query->fetchAll(PDO::FETCH_OBJ);
                $this->_count = $this->_query->rowCount();
            } else {
                $this->_error = true;
            }
        }

        return $this;

    }

and this is how I am calling the method

$db->query("SELECT * FROM users WHERE username = :username AND id = :id ", array(':username' => 'sayantan94', ':id' => 1));

But when I print_r() the result set, I am getting an empty array. But if I do this

$db->query("SELECT * FROM users WHERE username = :username", array(':username' => 'sayantan94'));

or this

$db->query("SELECT * FROM users WHERE id = :id ", array(':id' => 1));

I am getting proper result. What is wrong with my code??

Answer 1

Aside from what is said in the other answer, most of your code is either useless or harmful. In fact, you need only these few lines

public function query($sql, $params = array())
{
    $query = $this->_pdo->prepare($sql);
    $query->execute($params);
    return $query; 
}

It will do everything your function does, but without that much code and without nasty side effects. Imagine you will run a nested query in a loop. What become your $this->_results variable after the second query execution? In such a function there should be no class variables related to a particular query.

Besides,

• no need to check the prepare result manually - PDO will throw an exception by itself
• no need for the _error variable as it's pretty useless, while PDO's own exception is way more useful and informative
• no need to bind in a loop - execute() can bind all parameters at once
• no need to test $params array - execute() will accept an empty array as well.
• no need to return fetchAll() restult - you can always do it later, by chaining it like this $data = $db->query($sql, $params)->fetchAll();
• no need for rowCount() - you can always just count the array returned from fetchAll()