I was brought in to help development on a CakePHP web application recently. One of the tasks should have been relatively simple - add an option to an admin page. In a well-designed application, this task should have taken a couple hours. Instead it took a couple days because I needed to figure out just what was going on.
The view file was being used by 3 different actions which had slightly similar views. At the beginning of development, I’m sure there were more similarities than differences. But over time, you need to have one action display something just a little bit different than the others. It’s tempting just to add some logic to the view and be done with it. The problem is, after a few times of doing this, you’ve created a monster that is very difficult for other developers to understand what is going on.
For example, one of the controller actions was passing the following variables into the view:
$this->set([
'groups' => $syllabus['Section'],
'groupType' => 'syllabus',
'groupName' => 'Chapter',
'nonLinked' => $syllabus['NonLinked'],
'nonLinkedNotice' => 'Notice text here',
'title' => $syllabus['Course']['name'],
'heading' => 'Syllabus',
'placeHolderParentName' => 'Other Pages in this Course',
'thisId' => $id,
'groupsChildName' => 'Lesson',
'detached' => $syllabus['Detached'],
'hasDetached' => true,
'addExistingText' => 'Lesson',
'canDetachNonlinked' => true,
'hideEmailCount' => true,
'timeOptions' => $this->Course->courseIntervalTimes()
]);
By separating the views into separate files, I was able to remove the highlighted variables from being set. If I wanted to reduce the variables even more, I could have just set the $syllabus
variable and then read the sub-arrays from within the view. But I didn’t think of that at the time.
Anytime you are passing more than a few variables into the view, warning bells should start going off that it is time to refactor.
Multiple logic (if-then) statements should also be a warning sign.
You should avoid view code like this:
if ($add) {
// do something
}
if ($hasDetached) {
// do something else
}
Just create separate view files. If there is common code used in both, separate that code into an element and include it.
Keep logic out of the view files as much as possible. It will result in faster development and less frustration for anyone who needs to read and understand the code in the future.