I have a class like the following:
class DreamsImagesStore
{
public $table = 'dreams_images';
public function insertNewDreamImage($dream_id, $pid)
{
try {
$values = array($dream_id, $pid);
$sth = $this->dbh->prepare("INSERT INTO {$this->table}
(dream_id, pid)
VALUES (?, ?)");
if($sth->execute($values)) {
return true;
}
} catch(PDOException $e) {
$this->errorLogger($e);
}
}
...
}
I'm going to be implementing a new class called InterestsImagesStore wherein the only differences between these classes will be the value of $table
, $dream_id
will be $interest_id
, and dream_id
in SQL
will be interest_id
.
I know there's a better way to do this, and I'll be implementing similar classes in the future which have such small differences.
What's the best object-oriented way to refactor my code to avoid duplication and increase maintainability?
Create an ImagesStore
base class:
class ImagesStore
{
// See comments about accessors below.
private $table;
private $id_column;
public function insertImage($id, $pid) {
try {
$values = array($id, $pid);
$table = $this->getTable();
$id_column = $this->getIdColumn();
$sth = $this->dbh->prepare("INSERT INTO {$table} ($id_column, pid) VALUES (?, ?)");
if ($sth->execute($values)) {
return true;
}
}
catch (PDOException $e) {
$this->errorLogger($e);
}
}
protected function __construct($table, $id_column) {
$this->table = $table;
$this->id_column = $id_column;
}
// These accessors are only required if derived classes need access
// to $table and $id_column. Declaring the fields "private" and providing
// "protected" getters like this prevents the derived classes from
// modifying these values which might be a desirable property of these
// fields.
protected function getTable() {return $this->table;}
protected function getIdColumn() {return $this->id_column;}
// More implementation here...
// Initialize $dbh to something etc.
// Provide "errorLogger" method etc.
}
And create DreamsImagesStore
and InterestsImagesStore
specializations:
class DreamsImagesStore extends ImagesStore {
public function __construct() {
parent::__construct('dreams_images', 'dream_id');
}
}
class InterestsImagesStore extends ImagesStore {
public function __construct() {
parent::__construct('interests_images', 'interest_id');
}
}
The original method insertNewDreamImage
can be renamed to insertImage
as it is really more general than the original name suggests.
Note that ImagesStore
can also be declared abstract
if you want to prevent direct instantiation of it.
An alternative approach that can be adopted is to not bother deriving classes from ImagesStore
at all and just instantiate it directly by making the __construct
method public
and calling it as follows:
$dreamsImagesStore = new ImagesStore("dreams_images", "dream_id");
Another approach might also be to implement a static factory method in ImagesStore
.
using the ImagesStore class created by Richard cook, this could also happen:
function FactoryLoadImageStore($imageStoreType)
{
switch($imageStoreType)
{
case "Interests":
return new ImageStore('interests_images', 'interest_id');
case "Dreams":
return new ImageStore('dreams_images', 'dreams_id');
default:
throw new Exception("ImageStore type $imageStoreType not found")
; }
}
or you could even get fancier and do something like
function FactoryLoadImageStore($imageStoreType)
{
$tableName = $imageStoreType . 's_images';
$idColumnName = $imageStoreType . 's_id';
$tableExists = false;
$sql= "Show tables like '$tableName' ";
foreach ($this->dbh->query($sql) as $row)
{
if ($row['tableName'] == $tableName)
{
$tableExists = true;
break;
}
}
if( !$tableExists)
{
throw new Exception ("No matching table exists for the requested image store $imageStoreType");
}
return new ImageStore( $tableName, $idColumnName);
}
call as follows
$dreamImageStore = ImageStore::FactoryLoadImageStore('dream');
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With