Object Paradigm for PHP, Practice in Design

Question

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 is in fact
*   a fine day to create a report, we should then check to see if the desired
*   report has already been created. If it has not, we will create and save it.
*/

abstract class filterReports
{

    protected $reportFilenames = array(); // Store all file names in array, because it's fun
    protected $reportDirs = array(
        'daily-orders' => 'reports/daily/orders/',
        'weekly-orders' => 'reports/weekly/orders/',
        'monthly-orders' => 'reports/monthly/orders/',
        'daily-volume' => 'reports/daily/volume/',
        'weekly-volume' => 'reports/weekly/volume/',
        'monthly-volume' => 'reports/monthly/volume/',
    ); // Folders where we plan to store these reports

    protected function createFilenames()
    {

        // Comprehensive Order Data
        $this->reportFilenames['daily-orders']      = 'store-report-'
                                                    . date('Ymd', strtotime('-1 day'))
                                                    . '.csv'; // Yesterday's Report
        $this->reportFilenames['weekly-orders']     = 'store-report-'
                                                    . date('Ymd', strtotime('-8 days'))
                                                    . '-'
                                                    . date('Ymd', strtotime('-1 day'))
                                                    . '.csv'; // Last 7 Days
        $this->reportFilenames['monthly-orders']    = 'store-report-'
                                                    . date('Ymd', strtotime('first day of last month'))
                                                    . '-'
                                                    . date('Ymd', strtotime('last day of last month'))
                                                    . '.csv'; // Last Month

        // General Product Volume Data
        $this->reportFilenames['daily-volume']      = 'store-volume-'
                                                    . date('Ymd', strtotime('-1 day'))
                                                    . '.csv'; // Yesterday's Report
        $this->reportFilenames['weekly-volume']     = 'store-volume-'
                                                    . date('Ymd', strtotime('-8 days'))
                                                    . '-'
                                                    . date('Ymd', strtotime('-1 day'))
                                                    . '.csv'; // Last 7 Days
        $this->reportFilenames['monthly-volume']    = 'store-volume-'
                                                    . date('Ymd', strtotime('first day of last month'))
                                                    . '-'
                                                    . date('Ymd', strtotime('last day of last month'))
                                                    . '.csv'; // Last Month

    }

    protected $reportsToCreate = array();   // Based on what day it is, a different report may need to be created

    protected function chooseReports()
    {

        $this->reportsToCreate['daily-orders'] = TRUE; // Because 'every day' occurs every day.
        $this->reportsToCreate['daily-volume'] = TRUE;

        if (date('N', time()) == '1') { // If today is Monday, create weekly report
            $this->reportsToCreate['weekly-orders'] = TRUE;
            $this->reportsToCreate['weekly-volume'] = TRUE;
        } else {
            $this->reportsToCreate['weekly-orders'] = FALSE;
            $this->reportsToCreate['weekly-volume'] = FALSE;
        }

        if (date('j', time()) == '1') { // If today is the first day of the month, create monthly report
            $this->reportsToCreate['monthly-orders'] = TRUE;
            $this->reportsToCreate['monthly-volume'] = TRUE;
        } else {
            $this->reportsToCreate['monthly-orders'] = FALSE;
            $this->reportsToCreate['monthly-volume'] = FALSE;
        }

    }

    protected $reportsExist = array(); // Now let's see which reports have already been created

    protected function searchReports()
    {

        foreach ($this->reportsToCreate as $key => $val) {

            if ($val != FALSE) {                
                if (!file_exists($this->reportDirs[$key] . $this->reportFilenames[$key])) {
                    $this->reportsExist[$key] = FALSE;
                } else {
                    $this->reportsExist[$key] = TRUE;
                }
            }           

        }

    }

}

manageReports.class.php

include('filterReports.class.php');

/*
*   As an extension of the previous class 'filterReports', if a desired report has
*   not been found, we will create and save it.
*/

class manageReports extends filterReports
{

    public $newReport;

    private $dbConfig = array();
    private $con;   

    private function dbParams() // This should all probably go somewhere else, but I haven't decided where just yet
    {

        $this->dbConfig = array(
            'host' => 'hostname',
            'user' => 'username',
            'pass' => 'password',
            'name' => 'database',
        );
        $this->con = mysql_connect(
            $this->dbConfig['host'],
            $this->dbConfig['user'],
            $this->dbConfig['pass']
        ) or die('MySQL Error: ' . mysql_errno() . ' - ' . mysql_error());

    }

    private function createDailyOrdersReport() // Collect data and build the report body
    {

        // Do things to create $dataHeading and $dataContent strings
        $this->newReport = $dataHeading . $dataContent;

    }

    private function createDailyVolumeReport()
    {
        $this->newReport = $dataHeading . $dataContent;
    }

    private function createWeeklyOrdersReport()
    {
        $this->newReport = $dataHeading . $dataContent;
    }

    private function createWeeklyVolumeReport()
    {
        $this->newReport = $dataHeading . $dataContent;
    }

    private function createMonthlyOrdersReport()
    {
        $this->newReport = $dataHeading . $dataContent;
    }

    private function createMonthlyVolumeReport()
    {
        $this->newReport = $dataHeading . $dataContent;
    }

    private function writeReport($key)
    {

        // Save the report to its appropriate folder
        $writeReportName = $this->reportDirs[$key] . $this->reportFilenames[$key]; // I don't know why this works, seems to me to be out of scope for these array fields
        $writeReportOpen = fopen($writeReportName, 'w');
        fwrite($writeReportOpen, $this->newReport) or die('Unable to write file: ' . $writeReportName);
        fclose($writeReportOpen);

    }

    public function createReports() // Finally, resolve which reports should be created - then create them.
    {

        parent::createFilenames();  // Create the report file names
        parent::chooseReports();    // Decide whether a report should be run
        parent::searchReports();    // Find out if the report exists already

        foreach ($this->reportsExist as $key => $val) { // Any reports that should be created today, T or F for 'exists'
            if ($val != TRUE) {
                if ($key = 'daily-orders') {
                    $this->createDailyOrdersReport();
                    $this->writeReport($key);
                }
                if ($key = 'daily-volume') {
                    $this->createDailyVolumeReport();
                    $this->writeReport($key);
                }
                if ($key = 'weekly-orders') {
                    $this->createWeeklyOrdersReport();
                    $this->writeReport($key);
                }
                if ($key = 'weekly-volume') {
                    $this->createWeeklyVolumeReport();
                    $this->writeReport($key);
                }
                if ($key = 'monthly-orders') {
                    $this->createMonthlyOrdersReport();
                    $this->writeReport($key);
                }
                if ($key = 'monthly-volume') {
                    $this->createMonthlyVolumeReport();
                    $this->writeReport($key);
                }
            }
        }

    }

}

Finally, I do this to make it go.

include('manageReports.class.php');

$initReports = new manageReports;
$initReports->createReports();

I preemptively appreciate any and all assistance you can provide as it will undoubtedly make me less bogus of a web developer.

EDIT: Another question I just thought of in after-thought; Should I even bother declaring my fields and methods in filterReports as protected, seeing as though this class cannot be instantiated in the first place?

EDIT: I have made some revisions to my code based on responses. For now, forget about the above two classes - Here is my new code. The first class instantiates each of my report generating sub-classes and executes a public function contained in each of them. The second class is a single report generation class.

manageReports.class.php

include('store/library/reports/dailyOrdersReport.class.php');

/*
* Having included the desired reporting classes,
* we now need a uniform process for access and
* execution of these classes.
*/

class manageReports
{

    private $reports = array();

    public function __construct()
    {

        $this->createReports();

    }

    private function createReports()
    {

        $this->createReport('dailyOrdersReport');

    }

    private function createReport($class)
    {

        $this->reports[] = new $class;

    }

    public function go()
    {

        foreach ($this->reports as $report) {
            $report->execReport();
        }

    }

}

dailyOrdersReport.class.php

include('store/library/dbConfig.class.php'); // May need these parameters for dependent methods

/*
* Validates the need for specified report creation,
* and if true - does so.
*/

class dailyOrdersReport extends dbConfig
{

    private $newReport; // Variable to store report data

    private $reportPath;
    private $reportName;

    private $reportTestResult = FALSE;

    public function __construct() // Preprocess validation answers, "Should we create this report?"
    {

        parent::__construct(); // MySQL Parameters

        $this->reportPath   = 'store/reports/daily/orders/';
        $this->reportName   = 'store-orders_'
                            . date('Ymd', strtotime('-1 day'))
                            . '.csv';
        $this->reportTest();

    }

    private function reportTest() // We don't need to test against the date for this report, just if it's already been created
    {

        if (!file_exists($this->reportPath . $this->reportName)) {
            $this->reportTestResult = TRUE;
        } else {
            return;
        }

    }

    private function createReport()
    {

        // About 100 lines of csv report generating madness
        $this->newReport = $dataHeading . $dataContent;

    }

    private function writeReport()
    {

        // Save the report to its appropriate folder
        $writeReportName = $this->reportPath . $this->reportName;
        $writeReportOpen = fopen($writeReportName, 'w');
        fwrite($writeReportOpen, $this->newReport) or die('Unable to write file: ' . $writeReportName);
        fclose($writeReportOpen);

    }

    public function execReport()
    {

        if ($this->reportTestResult) { // Evaluates to 'true' if report aught be generated and saved to file
            $this->createReport();
            $this->writeReport();
        } else {
            return;
        }

    }

}

Once again, the trigger.

error_reporting(E_ALL);

date_default_timezone_set('America/Chicago'); // For use by 'date()' and 'strtotime()'

include('store/library/manageReports.class.php');

$manageReports = new manageReports;
$manageReports->go();

Is this perfect yet, or are there still miles to go before I sleep?

Answer 1

When I see a bunch of repeated 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();

Answer 2

The first comment is that you have six functions in manageReports which are identical, which each call a seventh one, createReports. While I understand that you mean to have logical function naming, given that you have all the logic as to when each program should be made located in the chooseReports and createReports function, you might as well leave off all the unnecessary ones.

For a more general answer, it doesn't seem like these two classes need to be separate. Class length isn't the main thing to consider here; you want to group things intelligently. Broadly speaking, you subclass only when you're making a more specific instance of the superclass. In your case, if you find that different types of reports require different functionality, could subclass different types of reports.