HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

PDO wrapper class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
wrapperclasspdo

Problem

```
'SET NAMES utf8',
PDO::ATTR_PERSISTENT => true
);
try {
$this->_dbh = new PDO($dsn, $user, $pass, $options);
}
catch (PDOException $e) {
echo $e->getMessage();
exit();
}
}

public function query($query)
{
$this->_stmt = $this->_dbh->prepare($query);
}

public function bind($pos, $value, $type = null)
{
if (is_null($type)) {
switch (true) {
case is_int($value):
$type = PDO::PARAM_INT;
break;
case is_bool($value):
$type = PDO::PARAM_BOOL;
break;
case is_null($value):
$type = PDO::PARAM_NULL;
break;
default:
$type = PDO::PARAM_STR;
}
}
$this->_stmt->bindValue($pos, $value, $type);
}

public function execute()
{
$this->_queryCounter++;
return $this->_stmt->execute();
}

public function resultset()
{
$this->execute();
return $this->_stmt->fetchAll(PDO::FETCH_ASSOC);
}

public function single()
{
$this->execute();
return $this->_stmt->fetch(PDO::FETCH_ASSOC);
}

// returns last insert ID
//!!!! if called inside a transaction, must call it before closing the transaction!!!!!!
public function lastInsertId()
{
return $this->_dbh->lastInsertId();
}

// begin transaction // must be innoDatabase table
public function beginTransaction()
{
return $this->_dbh->beginTransaction();
}

// end transaction
public function endTransaction()
{
return $this->_dbh->commit();
}

// cancel transaction
public function cancelTransaction()
{
return $this->_dbh->rollBack();
}

// returns number of rows updated, deleted

Solution

I am making some minor comments on this now old question.

The way you have made your wrapper hard-codes your choices unnecessarily.

Observe your current constructor:

public function __construct($user, $pass, $dbname)
{
    $dsn = 'mysql:host=localhost;dbname=' . $dbname;
    //$dsn = 'sqlite:myDatabase.sq3';
    //$dsn = 'sqlite::memory:';
    $options = array(
                PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8',
                PDO::ATTR_PERSISTENT => true
                );
    try {
        $this->_dbh = new PDO($dsn, $user, $pass, $options);
    }
    catch (PDOException $e) {
        echo $e->getMessage();
        exit();
    }
}


It is clear from your comments that you are already considering a different dsn. You are needlessly locking PDO down to mysql localhost connections. All of these settings make your class less reusable.

Catching and calling exit further locks your class down to self-destructing. It gives no chance for someone to use your class, see that the database didn't connect catch the exception themselves and deal with it the way they want.

What does your class need?

It needs a PDO object (that is all). The rest of the class does not depend on all of the settings that you made in your constructor.

So, I would construct your PDO object outside of this class. Your constructor then becomes:

public function __constructor(PDO $pdo)
{
    $this->_dbh = $pdo;
}


Flexibility and Reusability ensues.

Code Snippets

public function __construct($user, $pass, $dbname)
{
    $dsn = 'mysql:host=localhost;dbname=' . $dbname;
    //$dsn = 'sqlite:myDatabase.sq3';
    //$dsn = 'sqlite::memory:';
    $options = array(
                PDO::MYSQL_ATTR_INIT_COMMAND => 'SET NAMES utf8',
                PDO::ATTR_PERSISTENT => true
                );
    try {
        $this->_dbh = new PDO($dsn, $user, $pass, $options);
    }
    catch (PDOException $e) {
        echo $e->getMessage();
        exit();
    }
}
public function __constructor(PDO $pdo)
{
    $this->_dbh = $pdo;
}

Context

StackExchange Code Review Q#3806, answer score: 3

Revisions (0)

No revisions yet.