I am currently building a small application that allows users to create a slideshow, each slideshow consisting of slides that are either videos or images. Although this will be relatively small at this point, I want to build it using best practices and keeping in mind that this project will no doubt grow.
Before I get too far setting up my classes, I want to make sure my architecture is sound, as I am relatively inexperienced with objected oriented PHP but understand the importance of setting things up right in the first place.
Firstly - my PDO connection and class autoloading takes place in my config file, whilst also defining a few constants, one for development mode, one for ensuring direct access to included files is prevented. So, config.php...
<?php
define('DEVELOPMENT_MODE', true);
ini_set('error_reporting', E_ALL);
ini_set('display_errors', 1);
/** Used to prevent direct access to included files */
define('DIRECT_ACCESS', true);
/** Define DB Constants */
define('DB_HOST', 'localhost');
define('DB_NAME', 'dbname');
define('DB_USER', 'dbuser');
define('DB_PASS', 'dbpass');
/**
* Connects to Database
*
* @return object PDO Instance
*/
function dbh_connect()
{
try {
$dbh = new PDO('mysql:host=' . DB_HOST . ';dbname=' . DB_NAME, DB_USER, DB_PASS);
$dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
} catch (PDOException $e) {
if (defined('DEVELOPMENT_MODE')) {
echo $e->getMessage();
exit;
} else {
echo 'Sorry, something seems to have gone wrong, we are aware of the problem and are looking into it. Please come back later';
mail('myemail', 'Error Connecting to dbname Database', 'Error Connecting to dbname Database');
exit;
}
}
return $dbh;
}
spl_autoload_register(function($class)
{
include 'mydirectory/classes/' . strtolower($class) . '.class.php';
});
I then have my first class, Slide class. I pass the db instance via dependency injection and set the private $dbh property to a PDO instance in my constructor.
I have also created a save method. It works, so no problems there. However, I am very conscious that classes should have one responsibility, and am wondering if this method should be somewhere else, in a different class. As you can see, the method could be copied and pasted into another class and work out of the box which is 1) good because it is portable, but 2) bad because this would be repeating myself.
<?php
class Slide
{
private $id = null;
private $ordernum = null;
private $slide_type = null;
private $location_id = null;
private $status = null;
private $dbh = null;
public function __construct($dbh)
{
$this->dbh = $dbh;
}
public function __get($property)
{
if (property_exists($this, $property)) {
return $this->$property;
}
}
public function __set($property, $value) {
if (property_exists($this, $property)) {
$this->$property = $value;
}
}
public function save()
{
try {
$this->dbh->beginTransaction();
$properties = array_filter(get_object_vars($this));
unset($properties['dbh']);
unset($properties['id']);
$querystring = '';
foreach ($properties as $prop => $value) {
$querystring .= "{$prop} = :{$prop},";
}
$querystring = substr($querystring, 0, -1);
$stmt = $this->dbh->prepare("UPDATE slides SET " . $querystring . " WHERE id = :id");
foreach ($properties as $prop => $value) {
$stmt->bindValue(":{$prop}", $this->$prop);
}
$stmt->bindValue(':id', $this->id);
$stmt->execute();
$this->dbh->commit();
return $stmt->rowCount(); // if row count is 0 need to return error, trigger error, exeption???
} catch (PDOException $e) {
$this->dbh->rollBack();
if (defined('DEVELOPMENT_MODE')) {
echo $e->getMessage();
exit;
} else {
echo 'Sorry, something seems to have gone wrong, we are aware of the problem and are looking into it. Please come back later';
mail('myemail', 'Error Connecting to dbname Database', 'Error Connecting to dbname Database');
exit;
}
}
}
}
Is a save method like this acceptable to have in a specific class like the Slide class? If not and it should be in some sort of DAO, how would that work exactly?
A method that looks up a slide in the database and then maps the columns to object properties via PDO::FETCH_CLASS, would this also reside in this slide class?
Note: I understand that the __get and __set magic methods are slower than custom getters and setters, but I do like how compact the code is. Is there a risk when running PDO::FETCH_CLASS that these methods would cause problems if my column names do not match the properties of the class?