Sayantan Das Sayantan Das - 3 months ago 16
PHP Question

bindParam() not working properly

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 qurey 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

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()
Comments