patternphpMinor
Object Paradigm for PHP, Practice in Design
Viewed 0 times
practicephpdesignforparadigmobject
Problem
I've created and I manage a point of sale web application built in PHP which has thus far followed no clear guidelines or methodology for development; it's operation is completely procedural. In turn, because the department that's using it requests new and different features like it's a Las Vegas buffet, the software has become a mess which I'm terrified of (don't look it in the eyes). Thankfully, I'm the only developer and so no one else must feel the wrath of the beast I've created.
I've always had a hard time wrapping my head around OOP, but I think I'm finally beginning to understand the whole point behind encapsulating methods, protecting fields, and class inheritance. This brings me to my question: Given the object scheme posted below, am I doing this right? It works like it should and doesn't return any errors, but in terms of object design, I feel like a baby deer with wobbly legs, uncertain of the world around me.
To be a little more specific, should I have a separate class that encapsulates MySQL parameters - and where should it be included/inherited if many child classes, perhaps even on separate server requests, will need it?
Should these two classes be one? I thought to separate them for sake of file length - Is excessive file size a good indicator of when a class might need to be broken up?
Abstract, private, protected - I understand how this works in literal behavior, but in regard to use, I'm just swinging in the dark. Anyone care to shed a light on what I've done and whether it makes sense? I think that is the summation of my fears and concerns. Here is the code in question - Your replies will help guide my redesign/refactoring of everything I've spent the last 6 months on.
filterReports.class.php
```
date_default_timezone_set('America/Chicago'); // For use by 'date()' and 'strtotime()'
/*
* First, we will create our appropriate file names for the dates in question,
* then we will determine if today is a day to run said reports. If today
I've always had a hard time wrapping my head around OOP, but I think I'm finally beginning to understand the whole point behind encapsulating methods, protecting fields, and class inheritance. This brings me to my question: Given the object scheme posted below, am I doing this right? It works like it should and doesn't return any errors, but in terms of object design, I feel like a baby deer with wobbly legs, uncertain of the world around me.
To be a little more specific, should I have a separate class that encapsulates MySQL parameters - and where should it be included/inherited if many child classes, perhaps even on separate server requests, will need it?
Should these two classes be one? I thought to separate them for sake of file length - Is excessive file size a good indicator of when a class might need to be broken up?
Abstract, private, protected - I understand how this works in literal behavior, but in regard to use, I'm just swinging in the dark. Anyone care to shed a light on what I've done and whether it makes sense? I think that is the summation of my fears and concerns. Here is the code in question - Your replies will help guide my redesign/refactoring of everything I've spent the last 6 months on.
filterReports.class.php
```
date_default_timezone_set('America/Chicago'); // For use by 'date()' and 'strtotime()'
/*
* First, we will create our appropriate file names for the dates in question,
* then we will determine if today is a day to run said reports. If today
Solution
When I see a bunch of repeated
Here are the above requirements in an abstract base class, one designed to be extended by subclasses to fill out the specifics.
Here is an example subclass for one of the reports.
Hopefully this gives you a start on some OOness. :) I highly recommend the book Clean Code as it's a great help as you work to answer these questions for yourself.
Edit As you write the report classes, you may find that all orders reports share some functionality in common that volume reports don't and vice versa. If it's significant you may want to create more abstract classes
Of course what's missing now is a way to run the reports! The following is more procedural than OO, but it could be driven by a file or something similar.
if tests of a value against several constants, I think "make these classes." You have a Report base class screaming to get out with one subclass per report type. If you think of each report in a general way, you'll start to see what operations it needs to support:- Decide if it should be run given the date
- Check if it exists on disk
- Generate its file name and title
- Extract the data from MySQL into a text block
- Write itself to disk
Here are the above requirements in an abstract base class, one designed to be extended by subclasses to fill out the specifics.
abstract class AbstractReport {
private $directory;
private $date;
public function __construct($directory, $date) {
$this->directory = 'reports/' . $directory;
$this->date = $date;
}
public abstract function getTitle() ;
public abstract function getFileName() ;
public abstract function isNeeded() ;
public function hasBeenRun() {
return file_exists($this->key . $this->getFileName();
}
public function runIfNeeded() {
if ($this->isNeeded() && !$this->hasBeenRun()) {
$this->run();
}
}
public function run() {
$this->connectToDatabase();
file_put_contents($this->getTitle(), $this->buildReport());
}
protected function connectToDatabase() {
// ... mysql_connect() ...
}
protected abstract function buildReport() ;
protected function formatDate($offset, $format='Ymd') {
return date($format, strtotime($offset, $this->date));
}Here is an example subclass for one of the reports.
class DailyOrdersReport extends AbstractReport {
public function __construct($date) {
parent::__construct('daily/orders/', $date);
}
public function getTitle() {
return 'Daily Orders';
}
public function getFileName() {
return 'store-report-' . $this->formatDate('-1 day');
}
public function isNeeded() {
return true; // or use $this->date to make determination
}
protected abstract function buildReport() {
// ... pull data from database and return formatted text ...
}
}Hopefully this gives you a start on some OOness. :) I highly recommend the book Clean Code as it's a great help as you work to answer these questions for yourself.
Edit As you write the report classes, you may find that all orders reports share some functionality in common that volume reports don't and vice versa. If it's significant you may want to create more abstract classes
AbstractOrdersReport and AbstractVolumeReport. If the only difference between the time frames is the dates passed to the database queries, you could gain a lot from this.Of course what's missing now is a way to run the reports! The following is more procedural than OO, but it could be driven by a file or something similar.
class ReportManager {
private $reports = array();
public function __construct($date) {
$this->date = $date;
$this->createReports();
}
public function createReports() {
// could read these from disk or a table
$this->createReport('DailyOrders');
$this->createReport('DailyVolume');
$this->createReport('WeeklyOrders');
$this->createReport('WeeklyVolumn');
$this->createReport('MonthlyOrders');
$this->createReport('MonthlyVolumn');
}
protected function createReport($class) {
$this->reports[] = new $class($this->date);
}
public function runIfNeeded() {
foreach ($this->reports as $report) {
$report->runIfNeeded();
}
}
public function run() {
foreach ($this->reports as $report) {
$report->run();
}
}
}
// ... and to kick it off ...
$date = time();
$manager = new ReportManager();
$manager->runIfNeeded();Code Snippets
abstract class AbstractReport {
private $directory;
private $date;
public function __construct($directory, $date) {
$this->directory = 'reports/' . $directory;
$this->date = $date;
}
public abstract function getTitle() ;
public abstract function getFileName() ;
public abstract function isNeeded() ;
public function hasBeenRun() {
return file_exists($this->key . $this->getFileName();
}
public function runIfNeeded() {
if ($this->isNeeded() && !$this->hasBeenRun()) {
$this->run();
}
}
public function run() {
$this->connectToDatabase();
file_put_contents($this->getTitle(), $this->buildReport());
}
protected function connectToDatabase() {
// ... mysql_connect() ...
}
protected abstract function buildReport() ;
protected function formatDate($offset, $format='Ymd') {
return date($format, strtotime($offset, $this->date));
}class DailyOrdersReport extends AbstractReport {
public function __construct($date) {
parent::__construct('daily/orders/', $date);
}
public function getTitle() {
return 'Daily Orders';
}
public function getFileName() {
return 'store-report-' . $this->formatDate('-1 day');
}
public function isNeeded() {
return true; // or use $this->date to make determination
}
protected abstract function buildReport() {
// ... pull data from database and return formatted text ...
}
}class ReportManager {
private $reports = array();
public function __construct($date) {
$this->date = $date;
$this->createReports();
}
public function createReports() {
// could read these from disk or a table
$this->createReport('DailyOrders');
$this->createReport('DailyVolume');
$this->createReport('WeeklyOrders');
$this->createReport('WeeklyVolumn');
$this->createReport('MonthlyOrders');
$this->createReport('MonthlyVolumn');
}
protected function createReport($class) {
$this->reports[] = new $class($this->date);
}
public function runIfNeeded() {
foreach ($this->reports as $report) {
$report->runIfNeeded();
}
}
public function run() {
foreach ($this->reports as $report) {
$report->run();
}
}
}
// ... and to kick it off ...
$date = time();
$manager = new ReportManager();
$manager->runIfNeeded();Context
StackExchange Code Review Q#776, answer score: 8
Revisions (0)
No revisions yet.