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

Your Common Sense · Accepted Answer · 2016-08-29T11:22:18.183

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

@SayantanDas please check my article for the in-depth explanations of some bad practices you were using, [Your first database wrapper's childhood diseases](https://phpdelusions.net/pdo/common_mistakes) — Your Common Sense, Aug 29 '16 at 11:11

score 1 · Answer 2 · answered Aug 29 '16 at 10:58

1

You foreach is false, $value by value, because bindParam needs &$variable, Make a reference, try this:

if(count($params)) {
   foreach($params as $param => &$value) {
       $this->_query->bindParam($param, $value);
   }
}

http://php.net/manual/en/pdostatement.bindparam.php

answered Aug 29 '16 at 10:58

Provie9

379
2
14

i am getting this error `Warning: PDOStatement::bindParam(): SQLSTATE[HY093]: Invalid parameter number: Columns/Parameters are 1-based in D:\xampp\htdocs\oop\classes\DB.php on line 50` – Sayantan Das Aug 29 '16 at 11:03

BindParam() not working properly

2 Answers2