3
\$\begingroup\$

I've been building a PDO PHP DB class and I just wonder if someone could give me some feedback (regarding security, how the code is written and how I can optimize it).

This class is not finished function wise, but I would like the code I have currently to be reviewed.

<?php

Class DB {

    public static $init = null;

    private $_pdo       = null,
            $_query     = null,
            $_sql       = null,
            $_bounds    = null, // array
            $_error     = false,
            $_select    = array(
                        "SQL" => null,
                        "BOUNDS" => null),
            $_insert    = array(
                        "SQL" => null,
                        "BOUNDS" => null),
            $_update    = array(
                        "SQL" => null,
                        "BOUNDS" => null),
            $_delete    = array(
                        "SQL" => null,
                        "BOUNDS" => null),
            $_where     = null,
            $_results   = null, // array
            $_countRow  = false,
            $_countCol  = false;


    public static function init() {
        if(!isset(static::$init)) {
            static::$init = new DB();
        }
        return static::$init;
    }

    private function __construct() {
        try {
            $this->_pdo = new PDO('mysql:host=' . Config::get('mysql/host') . ';dbname=' . Config::get('mysql/db'), Config::get('mysql/username'), Config::get('mysql/password'));
            $this->_pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
            $this->_pdo->setAttribute(PDO::ATTR_DEFAULT_FETCH_MODE, PDO::FETCH_OBJ);
        } catch(PDOException $e) {
            $this->_error = $e->getMessage();
                return $this->_error;
        }
    }

    public function query() {

    }

    public function prepare() {
        // create sql-string
        $this->_select["SQL"] == null ? $this->_sql .= $this->_select["SQL"] : $this->_sql .= "{$this->_select['SQL']}; ";
        $this->_insert["SQL"] == null ? $this->_sql .= $this->_insert["SQL"] : $this->_sql .= "{$this->_insert['SQL']}; ";
        $this->_update["SQL"] == null ? $this->_sql .= $this->_update["SQL"] : $this->_sql .= "{$this->_update['SQL']}; ";
        $this->_delete["SQL"] == null ? $this->_sql .= $this->_delete["SQL"] : $this->_sql .= "{$this->_delete['SQL']}; ";

        // create bounds-array()
        $selects = array();
        $inserts = array();
        $updates = array();
        $deletes = array();

        if(!$this->_select["BOUNDS"] == null) {
            $selects = explode(", ", $this->_select["BOUNDS"]);
        }
        if(!$this->_insert["BOUNDS"] == null) {
            $inserts = explode(", ", $this->_insert["BOUNDS"]);
        }
        if(!$this->_update["BOUNDS"] == null) {
            $updates = explode(", ", $this->_update["BOUNDS"]);
        }
        if(!$this->_delete["BOUNDS"] == null) {
            $deletes = explode(", ", $this->_delete["BOUNDS"]);
        }

        $this->_bounds = array_merge($selects, $inserts, $updates, $deletes);

        if($this->_query = $this->_pdo->prepare($this->_sql)) {
            if(!$this->_bounds == null) {
            // check if bounds array have empty elements and delete it
            $x = 0;

            foreach($this->_bounds as $bounds) {
                if(empty($this->_bounds[$x])) {
                    unset($this->_bounds[$x]);
                }
                $x++;
            }

                $x = 1;

                foreach($this->_bounds as $keys) {
                    $this->_query->bindValue($x, $keys);
                    $x++;
                }
            } else {
                $this->_feedback = "Fail in Method: BOUNDS()";
            }
        }
        return $this;
    }

    public function execute() {
        if($this->_query->execute()) {
            if(substr($this->_select["SQL"], 0, 6) == "SELECT") {
                // finding amount of SELECTS in prepare query
                $check = explode("; ", $this->_select["SQL"]);
                $count = count($check);
                $x = 0;

                while($count > $x) {
                    // fetching all
                    $this->_results[] = $this->_query->fetchAll();
                    $this->_query->nextRowset();
                    $x++;
                }
                $this->_countRow = $this->_query->rowCount();
                $this->_countCol = $this->_query->columnCount();
            }
            return $this;
        }
    }

    public function select($table, $what = false) {
        $this->_select["BOUNDS"] == null ? $this->_select["BOUNDS"] : $this->_select["BOUNDS"] .= ", ";
        $this->_select["SQL"] == null ? $this->_select["SQL"] : $this->_select["SQL"] .= "; ";

        $what == false ? $what = "*" : $what;
        $this->_select["SQL"] .= "SELECT {$what} FROM {$table}";
        $this->_where = "SELECT";
            return $this;
    }

    public function insert($table, $fields = array()) {
        $this->_insert["BOUNDS"] == null ? $this->_insert["BOUNDS"] : $this->_insert["BOUNDS"] .= ", ";
        $this->_insert["SQL"] == null ? $this->_insert["SQL"] : $this->_insert["SQL"] .= "; ";

        $keys   = array_keys($fields);
        $values = null;
        $x      = 1;

        foreach($fields as $value) {
            $values .= "?";
            if($x < count($fields)) {
                $values .= ", ";
            }
            $x++;
        }

        $this->_insert["SQL"] .= "INSERT INTO {$table} (`" . implode('`, `', $keys) . "`) VALUES ({$values})";
        $this->_insert["BOUNDS"] .= implode(", ", array_values($fields));
            return $this;
    }

    public function update($table, $fields = array()) {
        $this->_update["BOUNDS"] == null ? $this->_update["BOUNDS"] : $this->_update["BOUNDS"] .= ", ";
        $this->_update["SQL"] == null ? $this->_update["SQL"] : $this->_update["SQL"] .= "; ";

        $set    = null;
        $x      = 1;

        foreach($fields as $name => $value) {
            $set .= "{$name} = ?";
            $this->_update["BOUNDS"] .= "{$value}, ";
            if($x < count($fields)) {
                $set .= ", ";
            }
            $x++;
        }

        $this->_update["SQL"] .= "UPDATE {$table} SET {$set}";
        $this->_where = "UPDATE";
            return $this;
    }

    public function delete($table, $fields = array()) {
        $this->_delete["SQL"] == null ? $this->_delete["SQL"] : $this->_delete["SQL"] .= "; ";
        $this->_delete["BOUNDS"] == null ? $this->_delete["BOUNDS"] : $this->_delete["BOUNDS"] .= ", ";

        $this->_delete["SQL"] .= "DELETE FROM {$table}";
        $this->_where = "DELETE";
            return $this;
    }

    public function where($fields = array(), $what = false) {
        $what == false ? $what = " AND " : $what = " {$what} ";
        $x = 1;
        $operators = array("<", "<=", "=", ">=", ">");

        if($this->_where == "SELECT") {
            $this->_select["SQL"] .= " WHERE ";
        } elseif($this->_where == "UPDATE") {
            $this->_update["SQL"] .= " WHERE ";
        } elseif($this->_where == "DELETE") {
            $this->_delete["SQL"] .= " WHERE ";
        }

        foreach($fields as $field) {
            $key        = $field[0];
            $operator   = $field[1];
            $value      = $field[2];

            if(in_array($operator, $operators)) {
                if($this->_where == "SELECT") {
                    $this->_select["SQL"] .= "{$key} {$operator} ?";
                    $this->_select["BOUNDS"] .= $value;

                    if($x < count($fields)) {
                        $this->_select["SQL"] .= $what;
                        $this->_select["BOUNDS"] .= ", ";
                    }

                } elseif($this->_where == "UPDATE") {
                    $this->_update["SQL"] .= "{$key} {$operator} ?";
                    $this->_update["BOUNDS"] .= $value;

                    if($x < count($fields)) {
                        $this->_update["SQL"] .= $what;
                        $this->_update["BOUNDS"] .= ", ";
                    }

                } elseif($this->_where == "DELETE") {
                    $this->_delete["SQL"] .= "{$key} {$operator} ?";
                    $this->_delete["BOUNDS"] .= $value;

                    if($x < count($fields)) {
                        $this->_delete["SQL"] .= $what;
                        $this->_delete["BOUNDS"] .= ", ";
                    }
                }
            }
            $x++;
        }
        return $this;
    }

    public function andWhere($fields = array(), $what = false) {
        $what == false ? $what = " AND " : $what = " {$what} ";
        $x = 1;
        $operators = array("<", "<=", "=", ">=", ">");

        if($this->_where == "SELECT") {
            $this->_select["SQL"] .= " AND ";
            $this->_select["BOUNDS"] .= ", ";
        } elseif($this->_where == "UPDATE") {
            $this->_update["SQL"] .= " AND ";
            $this->_update["BOUNDS"] .= ", ";
        } elseif($this->_where == "DELETE") {
            $this->_delete["SQL"] .= " AND ";
            $this->_delete["BOUNDS"] .= ", ";
        }

        foreach($fields as $field) {
            $key        = $field[0];
            $operator   = $field[1];
            $value      = $field[2];

            if(in_array($operator, $operators)) {
                if($this->_where == "SELECT") {
                    $this->_select["SQL"] .= "{$key} {$operator} ?";
                    $this->_select["BOUNDS"] .= $value;

                    if($x < count($fields)) {
                        $this->_select["SQL"] .= $what;
                        $this->_select["BOUNDS"] .= ", ";
                    }

                } elseif($this->_where == "UPDATE") {
                    $this->_update["SQL"] .= "{$key} {$operator} ?";
                    $this->_update["BOUNDS"] .= $value;

                    if($x < count($fields)) {
                        $this->_update["SQL"] .= $what;
                        $this->_update["BOUNDS"] .= ", ";
                    }

                } elseif($this->_where == "DELETE") {
                    $this->_delete["SQL"] .= "{$key} {$operator} ?";
                    $this->_delete["BOUNDS"] .= $value;

                    if($x < count($fields)) {
                        $this->_delete["SQL"] .= $what;
                        $this->_delete["BOUNDS"] .= ", ";
                    }
                }
            }
            $x++;
        }
        return $this;
    }

    public function orWhere($fields = array(), $what = false) {
        $what == false ? $what = " AND " : $what = " {$what} ";
        $x = 1;
        $operators = array("<", "<=", "=", ">=", ">");

        if($this->_where == "SELECT") {
            $this->_select["SQL"] .= " OR ";
            $this->_select["BOUNDS"] .= ", ";
        } elseif($this->_where == "UPDATE") {
            $this->_update["SQL"] .= " OR ";
            $this->_update["BOUNDS"] .= ", ";
        } elseif($this->_where == "DELETE") {
            $this->_delete["SQL"] .= " OR ";
            $this->_delete["BOUNDS"] .= ", ";
        }

        foreach($fields as $field) {
            $key        = $field[0];
            $operator   = $field[1];
            $value      = $field[2];

            if(in_array($operator, $operators)) {
                if($this->_where == "SELECT") {
                    $this->_select["SQL"] .= "{$key} {$operator} ?";
                    $this->_select["BOUNDS"] .= $value;

                    if($x < count($fields)) {
                        $this->_select["SQL"] .= $what;
                        $this->_select["BOUNDS"] .= ", ";
                    }

                } elseif($this->_where == "UPDATE") {
                    $this->_update["SQL"] .= "{$key} {$operator} ?";
                    $this->_update["BOUNDS"] .= $value;

                    if($x < count($fields)) {
                        $this->_update["SQL"] .= $what;
                        $this->_update["BOUNDS"] .= ", ";
                    }

                } elseif($this->_where == "DELETE") {
                    $this->_delete["SQL"] .= "{$key} {$operator} ?";
                    $this->_delete["BOUNDS"] .= $value;

                    if($x < count($fields)) {
                        $this->_delete["SQL"] .= $what;
                        $this->_delete["BOUNDS"] .= ", ";
                    }
                }
            }
            $x++;
        }
        return $this;
    }

    public function results() {
        // return all
    }

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

    public function countRow() {
        // return numbers of rows
    }

    public function countCol() {
        // return numbers of cols
    }
}

To use:

The where(), andWhere() and orWhere() methods can be chained. You can use the where() on its own if that's all you need, but andWhere() and orWhere() need to be chained on where(). You can modify AND/OR inside where(), andWhere() and orWhere() (AND is default inside all of them).

All results from selects will be fetched inside execute() and saved in results array.

Some examples:

// SELECT:
select("users")->where(array(array("username", "=", "name"), array("password", "=", "12345")), "AND")->orWhere(array(array("username", "=", "name2"), array("password", "=", "12345")))->andWhere(array(array("email", "=", "[email protected]"), array("age", "=", "1")), "OR");
// SELECT * FROM users WHERE username = ? AND password = ? OR username = ? AND password = ? AND email = ? OR age = ?

select("users")->where(array(array("username", "=", "thename")));
// SELECT * FROM users WHERE username = ?

select("users");
// SELECT * FROM users


// INSERT:
insert("users", array("username" => "name", "password" => "12345"));
// INSERT INTO users (`username`, `password`) VALUES (?, ?)


// UPDATE:
update("users", array("username" => "newname", "password" => "12345"))->where(array(array("username", "=", "oldname"), array("password", "=", "1")))->andWhere(array(array("email", "=", "[email protected]"), array("email", "=", "[email protected]")), "OR")->orWhere(array(array("username", "=", "name2"), array("age", "=", "1")));
// UPDATE users SET username = ?, password = ? WHERE username = ? AND password = ? AND email = ? OR email = ? OR username = ? AND age = ?

update("users", array("username" => "newname")->where(array(array("username", "=", "oldname"), array("password", "=", "1")));
// UPDATE users SET username = ?, password = ? WHERE username = ? AND password = ?


// DELETE:
delete("users")->where(array(array("username", "=", "oldname"), array("password", "=", "1")))->andWhere(array(array("email", "=", "[email protected]"), array("email", "=", "[email protected]")), "OR")->orWhere(array(array("username", "=", "name2"), array("age", "=", "1")));
// DELETE FROM users WHERE username = ? AND password = ? AND email = ? OR email = ? OR username = ? AND age = ?

delete("users")->where(array(array("username", "=", "oldname"), array("password", "=", "1")));
// DELETE FROM users WHERE username = ? AND password = ?


// PREPARE AND EXECUTE
prepare()->execute();
// prepare() binding values and execute() executes everything above at once.
\$\endgroup\$
3
  • \$\begingroup\$ Generally when I find myself mixing AND and OR clauses, I'm also using parentheses. E.g. SELECT * FROM table WHERE col1 = v1 AND ( col2 = v2 OR col3 = v3 ). This might make orWhere and andWhere less useful than they first seem. \$\endgroup\$
    – Brythan
    Commented Nov 12, 2014 at 14:30
  • 1
    \$\begingroup\$ Basically, the best way to optimize your code is to delete it... honestly. I'm sorry, but "To gild refined gold, to paint the lily, to throw a perfume on the violet" could as well be followed by "to wrap PDO". What you're doing is labour intensive, but pointless as I've explained here, and on various other occasions \$\endgroup\$
    – Elias Van Ootegem
    Commented Nov 13, 2014 at 16:10
  • \$\begingroup\$ I have allready read a lot of your pdo feedbacks. So now I know that this code is useless! \$\endgroup\$
    – jeastogg
    Commented Nov 13, 2014 at 16:54

1 Answer 1

Reset to default
3
\$\begingroup\$

Keep it flexible and simple

I've recently written a similar class, so I know why you would want to do something like this. At first it seems much easier just to put the needed SQL in a string, and bind values or variables to that, but classes like these make complex statements easier to maintain and reuse.

The use of chaining is good.

As for the security, that seems fine to me. You bind ALL your parameters. Perhaps this is a bit too much? With your system you also cannot bind parameters later, or repeatedly. You basically hide away the binding in your class. I find this limiting, which is the opposite of what you should be aiming at.

In my class I don't hide away the bindings, I prepare the statement and allow for binding of values in two ways:

A: At the statement building stage:

public function where($conditions,$bindings = NULL)

an example would be:

$query->where('ProductID = :ProductID',array('ProductID'  => $productID))
      ->where('SupplierID = :SupplierID',array('SupplierID' => $supplierID));
$data = $query->execute();

B: At the statement execution stage:

  public function execute($bindings = NULL)

an example would be:

$query->where('ProductID = :ProductID')
      ->where('SupplierID = :SupplierID');
$data = $query->execute(array('ProductID'  => $productID,
                              'SupplierID' => $supplierID));

In this last example I can reuse the prepared statement multiple times with different value bindings. I've decided not to allow variable bindings. Note that I use named placeholders, so I don't have to worry about the order of the bindings.

I see a lot of repeated code in your class. This will only get worse when you futher complete your class. The cause is a design decision you made. You use a different array for each SQL command. Your array's contain the same information, you preform the same operations on them, so they are, and should be, the same. Use one array.

A point which should really be addressed is the fact that you've decided to use one class to make the connection to the database, build a statement and execute it. In PDO itself two classes are used for this, for a very good reason. You really don't want to open a new connection for each new statement, you should reuse the connection. I have two classes:

class database extends PDO
// make and maintain a connection to the database
{
  <....>
  public function __construct($dataSource)
  <....>
}

class query
// perform queries on a database
{
  protected $statement  = NULL;
  <....>
  public function __construct($db,$query = '')
  <....>
}

$accessBigData = ['host'     => 'localhost',
                  'type'     => 'mysql',
                  'database' => 'bigdata',
                  'username' => 'me!',
                  'password' => 'a_secret'];

$dbBigData = new database($accessBigData);
$query1    = new query($dbBigData);
$query2    = new query($dbBigData);

This way I can reuse the database connection for each new statement. Of course I have put each class in its own PHP file.

By putting the SQL in classes I have tried to simplify writing SQL without sacrificing too much flexibility. With my classes you can still write out the SQL in a string and use it directly or you can build a statement using methods of the query class.

You seemed to have concentrated on the bindings mainly, and you forgot about the other things your class should enable.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for the feedback! To be honest; I dont know pdo every well, so I made this to learn more about it! I will defenitly do some adjustments based on this. \$\endgroup\$
    – jeastogg
    Commented Nov 12, 2014 at 10:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.