0
\$\begingroup\$

I'm trying to conform to the Zend Coding Standard as well.

EDIT 2:

<?php

/**
 *    Input   :   Query request
 *    Output  :   A row or a list of rows
 *    Notes   :
 *
 *    Provides escaping and binding via PDO library
 *    Consolidates PDO to use a single calling mechanism
 *    Consolidates all queries to an associative array
 *    Mysqli available for those who wish to implement
 */

class Database extends OneDatabase
{
    private $dbResource;
    public function __construct()  
    {
        $this->dbResource = oneDatabase::_get();
    }
    public function getResource()
    {
        return $this->dbResource;
    }
    protected $_sqlArray = array(
        "signin_pass" => "SELECT pass FROM cr WHERE email=?",
        "signin_validate" => "SELECT email,pass FROM cr WHERE email=? AND pass=?",
        "signin_flname" => "SELECT flname FROM cr WHERE email=?",
        "signup_check" => "SELECT * FROM cr WHERE email=?",  
        "signup_insert" => "INSERT INTO cr VALUES (?,?,?)",
        "signup_site1" => "INSERT INTO bo VALUES ('Facebook','http://www.facebook.com','',?)",
        "signup_site2" => "INSERT INTO bo VALUES ('Twitter','http://www.twitter.com','',?)",
        "bookmark_delete" => "DELETE FROM bo WHERE name=? AND email=? LIMIT 1",
        "bookmark_insert" => "INSERT INTO bo VALUES (?, ?,'', ?)",
        "bookmark_model" => "SELECT * FROM bo WHERE email=? ORDER BY name ASC",
        "tweet_insert" => "INSERT INTO tw VALUES (?,?,?,?)",
        "tweet_model" => "SELECT * FROM tw ORDER BY time DESC LIMIT 7",
        "create_bookmark" => "CREATE TABLE bookmark(name VARCHAR(64), url VARCHAR(256), tag VARCHAR(256), id INT)",
        "create_credentials" => "CREATE TABLE credentials(id INT NOT NULL AUTO_INCREMENT, flname VARCHAR(60), email VARCHAR(32), pass VARCHAR(40), PRIMARY KEY(id))",
        "create_tweet" => "CREATE TABLE tweet(id INT NOT NULL AUTO_INCREMENT, flname VARCHAR(60), email VARCHAR(32), pass VARCHAR(40), PRIMARY KEY(id))"
    );
    public function _pdoQuery($fetchType, $queryType, $parameterArray) 
    {
        $query=$this->_sqlArray[$queryType];
        eval("\$query = \"$query\";");
        if($parameterArray==0)
        {
            $results = oneDatabase::$_database->query($query);
            return $results;
        }
        oneDatabase::$_database->quote($query);
        $pdoStatement = oneDatabase::$_database->prepare($query);
        $pdoStatement->execute($parameterArray);
        if($fetchType=='none')
        {
            $results=NULL;
        }
        else if($fetchType=='single')
        {
            $results = $pdoStatement->fetch(PDO::FETCH_ASSOC);
        }
        else if($fetchType=='multiple')
        {
            $results = $pdoStatement->fetchAll();
        }
        return $results;
    }
    protected function _mysqliQuery($aquery) 
    {
        return oneDatabase::$_database->query($query);
    }
}
\$\endgroup\$
3
\$\begingroup\$

The good thing about this code is your bracing style which is applied consistently and is visually appealing. Now on to some improvements:

This code does not implement a Database. (You have called the class Database, but it does not resemble something that looks like a database). This class does not really have a good purpose. If you are going to write a database class it should handle database type things. Like transactions or methods for select, insert and delete statements etc.

Some other things:

  • It is not easy to re-use the code.
  • It contains hard coded values.
  • It breaks up a logical separation of concerns for your program.
  • Don't use eval - it is not good for your application security: eval("\$query = \"$query\";");
  • oneDatabase::$_database can be replaced by $this->_database as you are extending that class.
  • mysqli is mixed in with PDO. Just use PDO.
  • databaseCopy is not required
  • method access should be specified (public, protected, private)

In summary, I think you should re-think your design.

\$\endgroup\$
  • \$\begingroup\$ I can use either one...but I chose oneDatabase::$_database becasue this lets me know that it is static. In general I use $this for objects and class_name:: for classes (i.e. static members), even though it is not required. This is preference, I don't see why it matters either way. \$\endgroup\$ – user7459 Nov 20 '11 at 5:57
  • \$\begingroup\$ databaseCopy has been removed. \$\endgroup\$ – user7459 Nov 20 '11 at 5:58
  • \$\begingroup\$ Is PDO standard in all PHP installations? I left mysqli in case it is not in a standard installation and someone needs to revert. \$\endgroup\$ – user7459 Nov 20 '11 at 5:59
  • \$\begingroup\$ Added in method access \$\endgroup\$ – user7459 Nov 20 '11 at 6:00
  • 2
    \$\begingroup\$ My idea of a database class is similar to: codereview.stackexchange.com/questions/5294/pdo-wrapper-class Once you have that Database class defined you would create an instance of it and use it (rather than extending from it). In this way objects are created that do specific things. Likewise when using your database class you should not extend from it, but rather use it as an object. Then you will see how much your object is doing for you. Right now it is an object that provides only one method. \$\endgroup\$ – Paul Nov 20 '11 at 6:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy