Feedback please, DB conveniance class extends PDO

[jixor]

This is a work in progress, its basically a conveniance extension to PDO. I'm just looking for people's thoughts. Obviously there are a few fairly self-explanatory classes that go along with this so I won't include them. Thanks in advance.

(My coding standard is Zend with C influence in case your curious)

 

Mainly I want feedback on DB::getDOM() as I'm not really satisfied with it and undecided what I want to change.

 

<?php

namespace MPL;

class DB extends \PDO
{

    /**
     * Flag to format JSON output as human readable
     *
     * @see MPL\DB::getJSON()
     * @var int
     */
    const JSON_READABLE = 1;



    /**
     * Constructor
     *
     * Initializes the connection.
     *
     * @see   PDO::__construct()
     * @param DB_Options $Options
     */
    public function __construct(DB_Options $Options)
    {

        switch($Options->type)
        {

        case 'mysql':
        case 'pgsql':
            $strDSN = "$Options->type:host=$Options->host;dbname=$Options->name";
            if ($Options->port)
                $strDSN .= ";$Options->port";
            break;

        default:
            throw new DB_Exception("Unsupported type, $Options->type");
            return;

        }

        parent::__construct($strDSN, $Options->user, $Options->pass);

    }



    /**
     * Execute an insert statement
     *
     * @see    PDO::query()
     * @param  string $strSQL The SQL statement
     * @param  bool   $bDeep  Set to true if the SQL doesn't start with INSERT
     *                        but still creates a new record.
     * @return int    Last insert ID, if there was one. Or NULL on fail.
     */
    public function insert($strSQL, $bDeep = false)
    {

        if (!$bDeep && substr(trim(strtoupper($strSQL)), 0, 6) != 'INSERT')
            throw new DB_Query_Exception(
                'MPL\DB::insert(): Require INSERT statement'
                );

        if($this->query($strSQL))
            return $this->lastInsertId();

        return null;

    }



    /**
     * Get results as object
     *
     * @see    PDO::query()
     * @param  string       $strSQL SQL statement
     * @return PDOStatement
     */
    public function queryObj($strSQL)
    {

        return $this->query($strSQL, self::FETCH_OBJ);

    }



    /**
     * Get results an XML format
     *
     * @see    \PDO::query()
     * @param  string $strSQL     The SQL statement.
     * @param  string $strRoot    The XML root element.
     * @param  string $strRow     The row element
     * @return string
     */
    public function getXML($strSQL)
    {
/*
see todo on getDOM regarding pointer to root node
        $Dom = new DOMDocument;

        if (!$this->getDOM($Dom, $strSQL, null, $strRoot, $strRow))
            return false;

        return $Dom->saveXML();
*/
    }



    /**
     * Execute An SQL Statement
     *
     * @todo   Able to specify a pointer to the root node to fill the results
     *         into.
     * @todo   Might be better to accept an array or structure of options. This
     *         may be the single argument or substitute $oRoot onwards.
     *         Backwards compatibility would be preferable.
     * @see    \PDO::query()
     * @param  DOMDocument $oDoc     The DOMDocument, or derrived object.
     * @param  string      $strSQL   The SQL statement.
     * @param  DOMElement  $oRoot    The root element, defaults to the
     *                               document's firstChild. Technically could
     *                               be a member of a different Doc?
     * @param  mixed       $mRes     The tag name of the container element,
     *                               defaults to 'dataset' for <dataset />. Or
     *                               a DOMElement attached to the DOMDocument.
     * @param  string      $strRow   The tag name of the result row element,
     *                               defaults to 'data' for <data />.
     * @param  string      $strResID The container elments id, optional.
     * @return bool
     */
    public function getDOM(\DOMDocument $Doc, $strSQL, \DOMElement $oRoot = null, $mRes = 'dataset', $strRow = 'data', $strResID = null)
    {

        $res = $this->query($strSQL, self::FETCH_OBJ);

        if ($res->columnCount() < 1)
            return false;



        if (is_string($mRes))
        {

            $oRes = $Doc->createElement($strRes);

            if (!$oRoot)
                $Doc->firstChild->appendChild($oRes);

            else
                $oRoot->appendChild($oRes);

        }
        elseif ($mRes instanceof \DOMElement)
        {

            $oRes = $mRes;

        }
        else
        {

            throw new DB_DOM_Exception(
                "MPL\DB::queryDOM() Fouth argument must be either a DOMElement"
                    . " or string name of an element"
                );

        }



        if ($strResID)
            $oRes->setAttribute('id', $strResID);

        foreach ($res as $row)
        {

            $oRow = $Doc->createElement($strRow);

            foreach($row as $e => $v)
            {
                $oCol = $Doc->createElement($e, $v);
                $oRow->appendChild($oCol);
            }

            $oRes->appendChild($oRow);

        }

        return true;

    }



    /**
     * Get Results As JSON
     *
     * @todo   Should also be possible to explode dsv columns like sets or
     *         compiled columns.
     * @param  string $strSQL       SQL statement
     * @param  int    $iFlags       Flags, only DB_Conn::JSON_READABLE
     * @return string
     */
    public function getJSON($strSQL, $iFlags = 0)
    {

        $bReadable = ($iFlags & self::JSON_READABLE);

        $res = $this->query($strSQL, parent::FETCH_ASSOC);

        if ($res->rowCount() < 1)
            return '[]';



        $out = $bReadable ? "[\n" : '[';

        foreach($res as $row)
        {

            $out .= $bReadable ? "\n\t{" : '{';

            foreach($row as $col => $val)
                $out .= ($bReadable ? "\n\t\t" : '') . "\"$col\":\"$val\",";

            $out = substr($out, 0, -1) . ($bReadable ? "\n\t}," : '},');

        }

        return substr($out, 0, -1) . ($bReadable ? "\n]" : ']');

    }



    /**
     * Get A Single Column From A Single Row
     *
     * Assumedly your SQL statement will fetch only a single column from a
     * single row, otherwise you will simply get the first column from the first
     * row.
     *
     * @param  string $strSQL The SQL statement.
     * @return string Data, or FALSE on failure.
     */
    public function getVar($strSQL)
    {

        if (!$oRes = $this->query($strSQL, self::FETCH_COLUMN, 1))
            return null;

        if ($oRes->rowCount() < 1)
            return null;

        $mResult = $oRes->fetchColumn();

        $oRes->closeCursor();

        return $mResult;

    }



    /**
     * Get A Single Row Object
     *
     * If multiple rows are selected only the first row will be returned.
     *
     * @param  string $strSQL  The SQL statement.
     * @param  int    $iFlags  Flags
     * @param  object $oObject Generic object to fill results into
     * @return string Data, or FALSE on failure.
     */
    public function getRow($strSQL, $iFlags = 0, $oObject = null)
    {

        if ($oObject)
            if (!($iFlags & self::FETCH_INTO))
                $iFlags = self::FETCH_INTO;

        elseif (!$iFlags)
            $iFlags = $iFlags | self::FETCH_OBJ;



        if ($oObject)
            $res = $this->query($strSQL, self::FETCH_INTO, $oObject);

        else
            $res = $this->query($strSQL, self::FETCH_OBJ);



        if ($res->columnCount() < 1)
            return false;



        foreach($res as $r)
            return $r;

    }



    public function getCol($strSQL)
    {
    }

}

[ignace]

IMO this is just bad practice. You are mixing business logic with representation (XML, JSON)

 

switch($Options->type)
        {

        case 'mysql':
        case 'pgsql':
            $strDSN = "$Options->type:host=$Options->host;dbname=$Options->name";
            if ($Options->port)
                $strDSN .= ";$Options->port";
            break;

        default:
            throw new DB_Exception("Unsupported type, $Options->type");
            return;

        }

 

There are design patterns for this kind of behavior. cf Adapter pattern (http://en.wikipedia.org/wiki/Adapter_pattern). Plus your constructor should never do real work only initialization. Your collaborators if any should be lazy-loaded (http://en.wikipedia.org/wiki/Lazy_loading).

 

What I also don't like is the usage of mixed hungarian notation. Don't use it on and off. You might aswell just drop it as it has no value in a dynamic programming language like PHP. $txt.. may aswell be an object.

[jixor]

I had wondered about making the DB class abstract and then creating a DB_MYSQL class. This seems like a logical approach. That said there is only one PDO. Where do you think the connection string should be constructed?

 

Regarding lazy loading I think that is a good idea, however I have also constructed a seperate container class that can hold a collection of connections, that functions to initialize the connections when required not when defined. Do you think I should still add it to DB?

 

What do you think of including the getDOM, getJSON, etc methods in the class?

 

I'm aware of how PHPs dynamic type system functions, however I have found hungarian notation to be quite useful in improving script readability. What I have done is not use it in any interfaces because I know many people would not like it. That said overall I'm still undecided if I will use it or not.

[ignace]

I had wondered about making the DB class abstract and then creating a DB_MYSQL class

 

First of that Db_MySQL should be Db_Adapter_MySQL (cf Adapter Pattern). Second you need something to build the appropriate adapter Db::factory(..) (cf Factory Pattern) I highly recommend you stop trying to build your own DBAL. Because:

 

1) pre-made solutions are already available (PDO, ODBC, Zend_Db, Doctrine 2)

2) They will turn up rather complex quite fast

 

What behavior do you wish to add to PDO?

[jixor]

My main reasoning is simply the conveniance methods such as queryDOM. I want to keep it as simple as possible.

[jixor]

So I just want to add more conveniance to PDO, I don't want to abstract.

[jixor]

Ok so I have removed the constructor but decided against the Adaptor pattern. DB is now simply PDO with conveniance helpers tacked on. There is a connection manager that simplifies creating a connection. Granted some of your valid complaints still haven't been addressed but I'm just working through it one thing at a time.

 

<?php

namespace MPL;

class DB_Conns
{

    /**
     * Connection already exists exception
     */
    const ALREADY_EXISTS_ERR = 1;

    /**
     * Connection doesn't exist exception
     */
    const DOESNT_EXIST_ERR   = 2;

    /**
     * Invalid adapter type exception
     */
    const INVALID_TYPE_ERR   = 3;

    /**
     * There is no default connection exception
     */
    const NO_DEFAULT_ERR     = 4;



    /**
     * DB Connections
     *
     * @var array Array of DB objects
     */
    protected $aConns             = array();

    /**
     * Default Connection
     *
     * @var DB Default DB connection, if specified
     */
    protected $oDefaultConnection = null;



    /**
     * Constructor
     *
     * Optionally initialize a default connection
     *
     * @param  string     $strConnName The connection's name, defaults to NULL.
     *                                 If this is specified the second argument
     *                                 must also be present.
     * @param  DB_Options $DB_Options  Options for the connection, defaults to
     *                                 NULL. If this is specified the first
     *                                 argument must not be NULL.
     */
    public function __construct($strConnName = null, DB_Options $DB_Options = null)
    {

        if ((string)$strConnName && $DB_Options)
            $this->connect($strConnName, $DB_Options, true);

    }



    /**
     * Create A Connection
     *
     * @param  string     $strConnName The connection's name.
     * @param  DB_Options $DB_Options  Options for the connection.
     * @return DB
     */
    public function connect($strConnName, DB_Options $DB_Options, $bDefault = false)
    {

        $strConnName = (string)$strConnName;

        if (isset($this->aConns[$strConnName]))
            throw new DB_Conn_Exception(
                "A connection named $strConnName already exists",
                self::ALREADY_EXISTS_ERR
                );

        switch($DB_Options->type)
        {

        case 'mysql':
        case 'pgsql':
            $strDSN = "$DB_Options->type:host=$DB_Options->host;dbname=$DB_Options->name";
            if ($DB_Options->port)
                $strDSN .= ";$DB_Options->port";
            break;

        default:
            throw new DB_Conns_Exception(
                "Unsupported type, $DB_Options->type",
                self::INVALID_TYPE_ERR
                );

        }

        $this->aConns[$strConnName] = new DB(
            $strDSN,
            $DB_Options->user,
            $DB_Options->pass
            );

        if ($bDefault)
            $this->oDefaultConnection = $$this->aConns[$strConnName];

        return $this->aConns[$strConnName];

    }



    /**
     * Destructor
     *
     * Close connections
     */
    public function __destruct()
    {

        foreach($this->aConns as $strConnName => $DB)
            $this->disconnect($strConnName);

    }



    /**
     * Close Connection
     *
     * @param  string     $strConnName The connection's name.
     * @return void
     */
    public function disconnect($strConnName)
    {

        $strConnName = (string)$strConnName;

        if (isset($this->aConns[$strConnName]))
            unset($this->aConns[$strConnName]);

        else
            throw new DB_Conns_Exception(
                "There is no connection named $strConnName",
                self::DOESNT_EXIST_ERR
                );

    }



    /**
     * Magic Get Connection
     *
     * @param  string     $strConnName The connection's name.
     * @return DB
     */
    public function __get($strConnName)
    {

        $this->get($strConnName);

    }



    /**
     * Get Connection
     *
     * @param  string     $strConnName The connection's name.
     * @return DB
     */
    public function get($strConnName)
    {

        $strConnName = (string)$strConnName;

        if (isset($this->aConns[$strConnName]))
            return $this->aConns[$strConnName];

        else
            throw new DB_Conns_Exception(
                "There is no connection named $strConnName",
                self::DOESNT_EXIST_ERR
                );

    }



    /**
     * Magic Call Method On Default Connection
     *
     * Threr must be an explicit default connection set.
     *
     * @param  string $strMethodName The method name.
     * @param  array  $aArguments    Arguments
     * @return mixed
     */
    public function __call($strMethodName, $aArguments)
    {

        if (!$this->oDefaultConnection)
            throw new DB_Conns_Exception(
                'There is no default connection',)
        return call_user_func_array(
            array($this->oDefaultConnection, $strMethodName),
            $aArguments
            );

    }



    /**
     * Magic Set Connection
     *
     * @param string $strConnName The connection's name
     * @param DB     $DB          The DB
     */
    public function __set($strConnName, DB $DB)
    {

        if (isset($this->aConns[$strConnName]))
            throw new DB_Conn_Exception(
                "A connection named $strConnName already exists",
                self::ALREADY_EXISTS_ERR
                );

        $this->aConns[$strConnName] = $DB;

    }

}

 

<?php

namespace MPL;

class DB extends \PDO
{

    /**
     * Flag to format JSON output as human readable
     *
     * @see MPL\DB::getJSON()
     * @var int
     */
    const JSON_READABLE = 1;



    /**
     * Execute an insert statement
     *
     * @see    PDO::query()
     * @param  string $strSQL The SQL statement
     * @param  bool   $bDeep  Set to true if the SQL doesn't start with INSERT
     *                        but still creates a new record.
     * @return int    Last insert ID, if there was one. Or NULL on fail.
     */
    public function insert($strSQL, $bDeep = false)
    {

        if (!$bDeep && substr(trim(strtoupper($strSQL)), 0, 6) != 'INSERT')
            throw new DB_Query_Exception(
                'MPL\DB::insert(): Require INSERT statement'
                );

        if($this->query($strSQL))
            return $this->lastInsertId();

        return null;

    }



    /**
     * Get results as object
     *
     * @see    PDO::query()
     * @param  string       $strSQL SQL statement
     * @return PDOStatement
     */
    public function queryObj($strSQL)
    {

        return $this->query($strSQL, self::FETCH_OBJ);

    }



    /**
     * Get results an XML format
     *
     * @see    \PDO::query()
     * @param  string $strSQL     The SQL statement.
     * @param  string $strRoot    The XML root element.
     * @param  string $strRow     The row element
     * @return string
     */
    public function getXML($strSQL)
    {
/*
see todo on getDOM regarding pointer to root node
        $Dom = new DOMDocument;

        if (!$this->getDOM($Dom, $strSQL, null, $strRoot, $strRow))
            return false;

        return $Dom->saveXML();
*/
    }



    /**
     * Execute An SQL Statement
     *
     * @todo   Able to specify a pointer to the root node to fill the results
     *         into.
     * @todo   Might be better to accept an array or structure of options. This
     *         may be the single argument or substitute $oRoot onwards.
     *         Backwards compatibility would be preferable.
     * @see    \PDO::query()
     * @param  DOMDocument $oDoc     The DOMDocument, or derrived object.
     * @param  string      $strSQL   The SQL statement.
     * @param  DOMElement  $oRoot    The root element, defaults to the
     *                               document's firstChild. Technically could
     *                               be a member of a different Doc?
     * @param  mixed       $mRes     The tag name of the container element,
     *                               defaults to 'dataset' for <dataset />. Or
     *                               a DOMElement attached to the DOMDocument.
     * @param  string      $strRow   The tag name of the result row element,
     *                               defaults to 'data' for <data />.
     * @param  string      $strResID The container elments id, optional.
     * @return bool
     */
    public function getDOM(\DOMDocument $Doc, $strSQL, \DOMElement $oRoot = null, $mRes = 'dataset', $strRow = 'data', $strResID = null)
    {

        $res = $this->query($strSQL, self::FETCH_OBJ);

        if ($res->columnCount() < 1)
            return false;



        if (is_string($mRes))
        {

            $oRes = $Doc->createElement($strRes);

            if (!$oRoot)
                $Doc->firstChild->appendChild($oRes);

            else
                $oRoot->appendChild($oRes);

        }
        elseif ($mRes instanceof \DOMElement)
        {

            $oRes = $mRes;

        }
        else
        {

            throw new DB_DOM_Exception(
                "MPL\DB::queryDOM() Fouth argument must be either a DOMElement"
                    . " or string name of an element"
                );

        }



        if ($strResID)
            $oRes->setAttribute('id', $strResID);

        foreach ($res as $row)
        {

            $oRow = $Doc->createElement($strRow);

            foreach($row as $e => $v)
            {
                $oCol = $Doc->createElement($e, $v);
                $oRow->appendChild($oCol);
            }

            $oRes->appendChild($oRow);

        }

        return true;

    }



    /**
     * Get Results As JSON
     *
     * @todo   Should also be possible to explode dsv columns like sets or
     *         compiled columns.
     * @param  string $strSQL       SQL statement
     * @param  int    $iFlags       Flags, only DB_Conn::JSON_READABLE
     * @return string
     */
    public function getJSON($strSQL, $iFlags = 0)
    {

        $bReadable = ($iFlags & self::JSON_READABLE);

        $res = $this->query($strSQL, parent::FETCH_ASSOC);

        if ($res->rowCount() < 1)
            return '[]';



        $out = $bReadable ? "[\n" : '[';

        foreach($res as $row)
        {

            $out .= $bReadable ? "\n\t{" : '{';

            foreach($row as $col => $val)
                $out .= ($bReadable ? "\n\t\t" : '') . "\"$col\":\"$val\",";

            $out = substr($out, 0, -1) . ($bReadable ? "\n\t}," : '},');

        }

        return substr($out, 0, -1) . ($bReadable ? "\n]" : ']');

    }



    /**
     * Get A Single Column From A Single Row
     *
     * Assumedly your SQL statement will fetch only a single column from a
     * single row, otherwise you will simply get the first column from the first
     * row.
     *
     * @param  string $strSQL The SQL statement.
     * @return string Data, or FALSE on failure.
     */
    public function getVar($strSQL)
    {

        if (!$oRes = $this->query($strSQL, self::FETCH_COLUMN, 1))
            return null;

        if ($oRes->rowCount() < 1)
            return null;

        $mResult = $oRes->fetchColumn();

        $oRes->closeCursor();

        return $mResult;

    }



    /**
     * Get A Single Row Object
     *
     * If multiple rows are selected only the first row will be returned.
     *
     * @param  string $strSQL  The SQL statement.
     * @param  int    $iFlags  Flags
     * @param  object $oObject Generic object to fill results into
     * @return string Data, or FALSE on failure.
     */
    public function getRow($strSQL, $iFlags = 0, $oObject = null)
    {

        if ($oObject)
            if (!($iFlags & self::FETCH_INTO))
                $iFlags = self::FETCH_INTO;

        elseif (!$iFlags)
            $iFlags = $iFlags | self::FETCH_OBJ;



        if ($oObject)
            $res = $this->query($strSQL, self::FETCH_INTO, $oObject);

        else
            $res = $this->query($strSQL, self::FETCH_OBJ);



        if ($res->columnCount() < 1)
            return false;



        foreach($res as $r)
            return $r;

    }



    public function getCol($strSQL)
    {
    }

}

 

[ignace]

getXML nor getDOM nor getJSON is a responsibility of your database. This should be delegated to a view helper.