I am very new to PHP Object Orientated Programming so was wondering if I could get some good advice on a database object I created.
I call the class db and include my class into every page load and initiate the database object using $db = new db. I then call the method inside this for each action I may need (building a menu from the database, getting login information etc.) with different parameters depending on what it is i want to do.
It takes its first parameter as the query with the ? symbol as replacements for values I want to bind, the second parameter is the values to bind to it in an array that is then looped through inside the prepared_statement method and the third parameter is the type ( FETCH_ARRAY returns an array of a SELECT statements rows, NUM_ROWS returns the amount of rows affected and INSERT returns the last inserted ID).
An example of how I would call this function is below:
$db->prepared_execute( "SELECT * FROM whatever WHERE ? = ? ", array( 'password', 'letmein' ), NUM_ROWS );
The second and third parameters are optional for if there are no parameters to be bound or no return is needed.
As I am new to OOP I am finding it hard to get my head around exactly when to use correctly and what are public, private, static functions/variables and design patterns (Singleton etc.).
I've read many tutorials to get as far as I hav but I feel now I need to take it here to get further answers or advice on where to go next with OOP and with this class I've built.
It works as it is which for me is a good starting place except for any error handling which I will add in next but I want to make sure I am not making any obvious design errors here.
The code for the class is below:
class db {
var $pdo;
public function __construct() {
$this->pdo = new PDO('mysql:dbname=' . DB_NAME . ';host=' . DB_HOST . ';charset=utf8', DB_USER, DB_PASS);
$this->pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
$this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}
public function prepared_execute( $query, $bind_values = null, $type = null ) {
$preparedStatement = $this->pdo->prepare( $query );
if( $bind_values ) {
$i = 1;
foreach( $bind_values as $bind_value ) {
$preparedStatement->bindValue($i, $bind_value);
$i++;
} }
$preparedStatement->execute();
if( $type == FETCH_ARRAY ) { return $preparedStatement->fetchAll(); }
elseif( $type == NUM_ROWS ) { return $preparedStatement->rowCount(); }
elseif( $type == INSERT ) { return $this->pdo->lastInsertId(); }
else{ return true; }
}
Your code is a bit outdated. You should use one of the visibility keywords instead of var to declare your properties. In this case you probably want to use protected so that it can't be modified from outside the class but so that any future sub-classes can modify it internally. You'll also probably want to add a getter in-case you need to work with PDO directly (which you will - see my final statements below my class example).
It's bad to hard code you PDO connection information in the class. You should pass these in as parameters same as you would if using PDO directly. I would also add the ability to pass in a pre-configured PDO instance as well.
While not required, its a good idea to conform to PSR-0 through PSR-2; Specifically in your case I'm speaking about Class and method naming which should both be camelCase and the first char of the class should be capital. Related to this, your code formatting is also ugly, particularly your block statements. If that's just an issue with copy and paste then ignore that comment.
So overall I would refactor your code to look something like this:
class Db {
protected $pdo;
public function __construct($dsn, $user, $pass, $options = array()) {
if($dsn instanceof PDO) {
// support passing in a PDO instance directly
$this->pdo = $dsn;
} else {
if(is_array($dsn)) {
// array format
if(!empty($options)) {
$dsn['options'] = $options;
}
$dsn = $this->buildDsn($options);
} else {
// string DSN but we need to append connection string options
if(!empty($options)) {
$dsn = $this->buildDsn(array('dsn' => $dsn, 'options' => $options));
}
}
// otherwise just use the string dsn
// ans create PDO
$this->pdo = new PDO($dsn, $user, $pass);
}
// set PDO attributes
$this->pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
$this->pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}
public function getConnection()
{
return $this->pdo;
}
protected function buildDsn($options) {
if($isDbParts = isset($options['dbname'], $options['hostname']) || !($isDsn = isset($option['dsn']))) {
throw new Exception('A dsn OR dbname and hostname are required');
}
if($isDsn === true) {
$dsn = $options['dsn'];
} else if {
$format = '%s:dbname=%s;host=%s';
$driver = isset($options['dbtype']) ? $options['dbtype'] : 'mysql';
$dsn = sprintf($format, $options['dbtype'], $options['dbname'], $options['host']);
}
if(isset($options['options'])) {
$opts = array();
foreach($options['options'] as $name => $value) {
$opts[] = $name . '=' . $value;
}
if(!empty($opts)) {
$dsn .= ';' . implode(';', $opts);
}
}
return $dsn;
}
public function preparedExecute( $query, $bind_values = null, $type = null ) {
$preparedStatement = $this->pdo->prepare( $query );
if( $bind_values ) {
$i = 1;
foreach( $bind_values as $bind_value ) {
$preparedStatement->bindValue($i, $bind_value);
$i++;
}
}
$preparedStatement->execute();
if( $type == FETCH_ARRAY ) {
return $preparedStatement->fetchAll();
}
elseif( $type == NUM_ROWS ) {
return $preparedStatement->rowCount();
}
elseif( $type == INSERT ) {
return $this->pdo->lastInsertId();
}
else {
return true;
}
}
}
Lastly, unless this is just for educational purposes I wouldn't do this. There are a ton of different queries that have varying part assemblies which aren't considered here so at some point this is not going to support what you need it to do. Instead I would use Doctrine DBAL, Zend_Db or something similar that is going to support greater query complexity through its API. In short, don't reinvent the wheel.