I'm coding a report builder in PHP. These reports will either be shown on a web page, emailed or sent through some other communication line. I'd like to support different types of report data (currently text and tabular).
I've built this and it works, the main classes are:
My concern is that my solution is not extensible, due to my implementation of ReportBuilder:
public function addTextReport(ITextReport $report)
{
$this->textReports[] = $report;
return $this;
}
public function addTabularReport(ITabularReport $report)
{
$this->tabularReports[] = $report;
return $this;
}
public function getData()
{
$data = ['tabularReports' => [], 'textReports' => []];
foreach($this->tabularReports as $report)
{
$data['tabularReports'][] = [
'title' => $report->getTitle(),
'keys' => $report->getDataTitles(),
'data' => $report->getData(),
];
}
foreach($this->textReports as $report)
{
$data['textReports'][] = [
'title' => $report->getTitle(),
'content' => $report->getContent(),
];
}
return $data;
}
My problem:
If I want to add a new report type (say IGraphReport
), I need to modify ReportBuilder
to accept this new report type, and then parse it in the ReportBuilder::getData
method. This violates the Open and Closed principle, and just seems wrong.
Some solutions I considered:
Creating a more generic interface IReport
, with a render
method on it, and replacing the addTextReport
and addTabularReport
in ReportBuilder
to a more generic addReport
. However, the different communication channels require different methods of rendering the data so the render
method somehow accepts instruction on how to format the data. I'm not sure if this is the cleanest solution.
if
statements checking the type of the report: if($report instanceof ITabularReport) { // handle }
, which would then lead me to "replace conditionals with polymorphism" and take me back to point 1.I'm not sure how to refactor. Any thoughts?
Having an addTextReport
and addTabularReport
method seems like you're tying yourself to the implementation logic. Why not just have an addReport
method?
Have each type of Report
adhere to a contract (interface) which implements the getData
method. I.e. delegate the responsibility of how the data is returned, to the class.
private $reports;
public function addReport(ReportContract $report)
{
$this->reports[] = $report;
return $this;
}
public function getData()
{
$data = [];
foreach($this->reports as $report) {
$data[] = $report->getData();
}
return $data;
}
interface ReportContract
{
public function getData();
}
class ITextReport implements ReportContract
{
public function getData()
{
// return some data
}
}
Now, each new type of report (Graph for example) simply has to implement a getData
method and your base ReportBuilder class requires no changes or refactoring to support it.