patternphplaravelMinor
Plotting a graph of survey answers in Laravel
Viewed 0 times
surveygraphanswersplottinglaravel
Problem
I'm using PHP (Laravel) to iterate over the
ChartFactory.php
survey_answers table and build datasets which I can show in a graph. I'm just wondering if this is the way to go.ChartFactory.php
public static function buildFromSurvey($survey)
{
$chart = new Chart($survey->name);
foreach($survey->answers as $answer) {
if($chart->hasDataset($answer->user_id)) {
$dataset = $chart->getDataset($answer->user_id);
} else {
$dataset = new Dataset();
$dataset->setTitle($answer->user->name);
$dataset->setColor(config('assessment.chart_colors_survey')[0]);
$dataset->setOwner($answer->user);
$chart->addDataSet($dataset);
}
// Create the category if not exists in this dataset
if ($dataset->hasDataItem($answer->question->category->name)) {
$category = $dataset->getDataItem($answer->question->category->name);
$category->addScore($answer->score);
} else {
$category = new Category();
$category->setName($answer->question->category->name);
$category->addScore($answer->score);
$dataset->addDataItem($category);
$chart->addLabel($answer->question->category->name);
}
}
return $chart;
}Solution
I think my main criticism of this code would be that it is asymmetric in terms of where ownership of instantiating your different object dependencies lives.
For example, in the first conditional around whether
This same concern exists for me with regards to creating
I would also strongly recommend using type hinting for the parameter to enforce that you are getting passed a valid survey object.
For example, in the first conditional around whether
Dataset exists on the Chart, why does this code need to own the logic to create and attach a new Dataset? If the Chart object "owns" the datasets, I would think that perhaps all this code would do is call a Chart::createNewDataset() or similar method to handle the work building a new dataset and setting it on the Chart.This same concern exists for me with regards to creating
Category objects.I would also strongly recommend using type hinting for the parameter to enforce that you are getting passed a valid survey object.
Context
StackExchange Code Review Q#156470, answer score: 2
Revisions (0)
No revisions yet.