PHP oop coding, please comment.

[Supervan]

Hi,

I'm trying to improve my coding and would appreciate if someone could comment and alter my code.
I dont know what would work best, named placeholders or question mark placeholders. Place the variables in a array first and then do the binding.

Im looking also for solutions regarding record update using place holders.

Thanks

    $idnrr = 1;
    $users = DB::getInstance()->query("SELECT * FROM users WHERE id = :id1", array(':id1' => $idnrr));
    if ($users->count()) {
        foreach ($users->results() as $result) {
            echo $result->name . "<br />";
        }
    }

public function query($sql, $params) {
        $this->_error = false;
        if ($this->_query = $this->_pdo->prepare($sql)) {
            $x = 1;
           

            if ($this->_query->execute($params)) {
                $this->_results = $this->_query->fetchAll(PDO::FETCH_OBJ);
                $this->_count = $this->_query->rowCount();
            } else {
                $this->_error = true;
            }
        }

        return $this;
    }

[mac_gyver]

imo, it doesn't matter if you use named place holders or ? place holders. you have to keep track of how many, what they are used for, and supply an input parameter for each one.

 

btw - your existing code will work for either type of place holder. for a ? place holder, your query calling statement would become - $users = DB::getInstance()->query("SELECT * FROM users WHERE id = ?", array($idnrr));

 

however, your current method of supplying the parameters in the ->execute($params) method won't work for things like LIMIT x,y clauses (i.e. you are dynamically getting the y value from the user), because using ->execute($params) treats all the parameters as strings, which produces a sql syntax error when used in a LIMIT clause.

 

a general purpose solution would need a way of (optionally) supplying the input type (in each element in your $params array) and would specifically bind each input by looping over the elements of the $params array. see the following example -

    public function query($query,$data_in=array()){
        if($data_in){
            // prepared query
            $stmt = parent::prepare($query); // this example extends the pdo class
            foreach($data_in as $arr){
                if(isset($arr[2])){
                    // type supplied
                    $stmt->bindValue($arr[0],$arr[1],$arr[2]);
                } else {
                    // no type supplied
                    $stmt->bindValue($arr[0],$arr[1]); // defaults to string type
                }
            }
            $stmt->execute();
        } else {
            // non-prepared query
            $stmt = parent::query($query);
        }

        // code to retrieve the result from the query....

    }

to specifically supply an integer data type, the $params array entry, corresponding to your supplied example, would look like - array(':id',$idnrr,PDO::PARAM_INT). to use a ? place holder, this would become - array(1,$idnrr,PDO::PARAM_INT)

[Supervan]

imo, it doesn't matter if you use named place holders or ? place holders. you have to keep track of how many, what they are used for, and supply an input parameter for each one.

 

btw - your existing code will work for either type of place holder. for a ? place holder, your query calling statement would become - $users = DB::getInstance()->query("SELECT * FROM users WHERE id = ?", array($idnrr));

 

however, your current method of supplying the parameters in the ->execute($params) method won't work for things like LIMIT x,y clauses (i.e. you are dynamically getting the y value from the user), because using ->execute($params) treats all the parameters as strings, which produces a sql syntax error when used in a LIMIT clause.

 

a general purpose solution would need a way of (optionally) supplying the input type (in each element in your $params array) and would specifically bind each input by looping over the elements of the $params array. see the following example -

    public function query($query,$data_in=array()){
        if($data_in){
            // prepared query
            $stmt = parent::prepare($query); // this example extends the pdo class
            foreach($data_in as $arr){
                if(isset($arr[2])){
                    // type supplied
                    $stmt->bindValue($arr[0],$arr[1],$arr[2]);
                } else {
                    // no type supplied
                    $stmt->bindValue($arr[0],$arr[1]); // defaults to string type
                }
            }
            $stmt->execute();
        } else {
            // non-prepared query
            $stmt = parent::query($query);
        }

        // code to retrieve the result from the query....

    }

to specifically supply an integer data type, the $params array entry, corresponding to your supplied example, would look like - array(':id',$idnrr,PDO::PARAM_INT). to use a ? place holder, this would become - array(1,$idnrr,PDO::PARAM_INT)

 

Thanks for your help..

[Supervan]

I really appreciated your help. I tried, but missing something.

 

Error... Warning: PDOStatement::bindValue() expects parameter 3 to be long

$idnrr = 1;
$name1 = "tom12345";

$users = DB::getInstance()->query("SELECT * FROM users WHERE id = :id AND name = :name",
                                   array(':id'=>$idnrr,PDO::PARAM_INT,':name'=>$name1,PDO::PARAM_STR));

if ($users->count()) {
    foreach ($users->results() as $result) {
        echo $result->name . "<br />";
    }
}

class DB {

    private static $_instance = null; // use $_ notation for private
    private $_pdo,
            $_query,
            $_error = false,
            $_results,
            $_count = 0;

  
    public function query($sql, $data_in = array()) {
        $this->_error = false;
        if ($data_in) {// prepared query
            $this->_query = $this->_pdo->prepare($sql); // this example extends the pdo class
            foreach ($data_in as $arr) {
                if (isset($arr[2])) {// type supplied
                    $this->_query->bindValue($arr[0], $arr[1], $arr[2]);
                } else {// no type supplied
                    $this->_query->bindValue($arr[0], $arr[1]); // defaults to string type
                }
            }
            if ($this->_query->execute()) {
                $this->_results = $this->_query->fetchAll(PDO::FETCH_OBJ);
                $this->_count = $this->_query->rowCount();
            } else {
                $this->_error = true;
            }
        } else {// non-prepared query
            $this->_query = $this->_pdo->prepare($sql);
            if ($this->_query->execute()) {
                $this->_results = $this->_query->fetchAll(PDO::FETCH_OBJ);
                $this->_count = $this->_query->rowCount();
            } else {
                $this->_error = true;
            }
        }// code to retrieve the result from the query....
        return $this;
    }


  

    public function results() {
        return $this->_results;
    }

    public function first() {
        return $this->_results[0];
    }

    public function error() {
        return $this->_error;
    }

    public function count() {
        return $this->_count;
    }

}

 

[mac_gyver]

the array structure would be - array(array(':id',$idnrr,PDO::PARAM_INT),array(':name',$name1,PDO::PARAM_STR))

 

or more clearly -

$parms = array();
$parms[] = array(':id',$idnrr,PDO::PARAM_INT);
$parms[] = array(':name',$name1,PDO::PARAM_STR);

// then use the $parms array as the second parameter in your query calling statement -
 $users = DB::getInstance()->query("SELECT * FROM users WHERE id = :id AND name = :name",$parms);

[Supervan]

 

the array structure would be - array(array(':id',$idnrr,PDO::PARAM_INT),array(':name',$name1,PDO::PARAM_STR))

 

or more clearly -

$parms = array();
$parms[] = array(':id',$idnrr,PDO::PARAM_INT);
$parms[] = array(':name',$name1,PDO::PARAM_STR);

// then use the $parms array as the second parameter in your query calling statement -
 $users = DB::getInstance()->query("SELECT * FROM users WHERE id = :id AND name = :name",$parms);

 

This worked like a charm... Thank you so much.

Now I need to try and solve the rest. Update/ Insert and delete..