Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.
Common duplication problems, and corresponding solutions are:
| 1 | <?php |
||
| 20 | class BlockPresenter extends BasePresenter |
||
| 21 | { |
||
| 22 | |||
| 23 | /** |
||
| 24 | * @var integer |
||
| 25 | */ |
||
| 26 | private $blockId = NULL; |
||
| 27 | |||
| 28 | /** |
||
| 29 | * @var Emailer |
||
| 30 | */ |
||
| 31 | private $emailer; |
||
| 32 | |||
| 33 | /** |
||
| 34 | * @var MeetingModel |
||
| 35 | */ |
||
| 36 | private $meetingModel; |
||
| 37 | |||
| 38 | /** |
||
| 39 | * @param BlockModel $model |
||
| 40 | * @param Emailer $emailer |
||
| 41 | */ |
||
| 42 | public function __construct(BlockModel $model, Emailer $emailer, MeetingModel $meetingModel) |
||
| 43 | { |
||
| 44 | $this->setModel($model); |
||
|
|
|||
| 45 | $this->setEmailer($emailer); |
||
| 46 | $this->setMeetingModel($meetingModel); |
||
| 47 | } |
||
| 48 | |||
| 49 | /** |
||
| 50 | * @return void |
||
| 51 | */ |
||
| 52 | public function renderNew() |
||
| 75 | |||
| 76 | /** |
||
| 77 | * @return void |
||
| 78 | */ |
||
| 79 | public function actionCreate() |
||
| 80 | { |
||
| 81 | $model = $this->getModel(); |
||
| 82 | $data = $this->getHttpRequest()->getPost(); |
||
| 83 | |||
| 84 | $this->setBacklink($data['backlink']); |
||
| 85 | $data['from'] = date('H:i:s', mktime($data['start_hour'], $data['start_minute'], 0, 0, 0, 0)); |
||
| 86 | $data['to'] = date('H:i:s', mktime($data['end_hour'], $data['end_minute'], 0, 0, 0, 0)); |
||
| 87 | $data['meeting'] = $this->getMeetingId(); |
||
| 88 | |||
| 89 | unset($data['start_hour']); |
||
| 90 | unset($data['end_hour']); |
||
| 91 | unset($data['start_minute']); |
||
| 92 | unset($data['end_minute']); |
||
| 93 | unset($data['backlink']); |
||
| 94 | |||
| 95 | try { |
||
| 96 | $this->guardToGreaterThanFrom($data['from'], $data['to']); |
||
| 97 | $result = $this->getModel()->create($data); |
||
| 98 | |||
| 99 | Debugger::log('Creation of block successfull, result: ' . json_encode($result), Debugger::INFO); |
||
| 100 | |||
| 101 | $this->flashMessage('Položka byla úspěšně vytvořena', 'ok'); |
||
| 102 | } catch(Exception $e) { |
||
| 103 | Debugger::log('Creation of block with data ' . json_encode($data) . ' failed, result: ' . $e->getMessage(), Debugger::ERROR); |
||
| 104 | |||
| 105 | $this->flashMessage('Creation of block failed, result: ' . $e->getMessage(), 'error'); |
||
| 106 | } |
||
| 107 | |||
| 108 | $this->redirect($this->getBacklink() ?: 'Block:listing'); |
||
| 109 | } |
||
| 110 | |||
| 111 | /** |
||
| 112 | * @param integer $id |
||
| 113 | * @return void |
||
| 114 | */ |
||
| 115 | public function actionUpdate($id) |
||
| 116 | { |
||
| 117 | $model = $this->getModel(); |
||
| 118 | $data = $this->getHttpRequest()->getPost(); |
||
| 119 | |||
| 120 | $this->setBacklink($data['backlink']); |
||
| 121 | $data['from'] = date('H:i:s', mktime($data['start_hour'], $data['start_minute'], 0, 0, 0, 0)); |
||
| 122 | $data['to'] = date('H:i:s', mktime($data['end_hour'], $data['end_minute'], 0, 0, 0, 0)); |
||
| 123 | $data['meeting'] = $this->getMeetingId(); |
||
| 124 | array_key_exists('display_progs', $data) ?: $data['display_progs'] = '1'; |
||
| 125 | |||
| 126 | unset($data['start_hour']); |
||
| 127 | unset($data['end_hour']); |
||
| 128 | unset($data['start_minute']); |
||
| 129 | unset($data['end_minute']); |
||
| 130 | unset($data['backlink']); |
||
| 131 | |||
| 132 | try { |
||
| 133 | $this->guardToGreaterThanFrom($data['from'], $data['to']); |
||
| 134 | $result = $this->getModel()->update($id, $data); |
||
| 135 | |||
| 136 | Debugger::log('Modification of block id ' . $id . ' with data ' . json_encode($data) . ' successfull, result: ' . json_encode($result), Debugger::INFO); |
||
| 137 | |||
| 138 | $this->flashMessage('Položka byla úspěšně upravena.', 'ok'); |
||
| 139 | } catch(Exception $e) { |
||
| 140 | Debugger::log('Modification of block id ' . $id . ' failed, result: ' . $e->getMessage(), Debugger::ERROR); |
||
| 141 | |||
| 142 | $this->flashMessage('Modification of block id ' . $id . ' failed, result: ' . $e->getMessage(), 'error'); |
||
| 143 | } |
||
| 144 | |||
| 145 | $this->redirect($this->getBacklink() ?: 'Block:listing'); |
||
| 146 | } |
||
| 147 | |||
| 148 | /** |
||
| 149 | * @param int $id |
||
| 150 | * @return void |
||
| 151 | */ |
||
| 152 | View Code Duplication | public function actionDelete($id) |
|
| 165 | |||
| 166 | /** |
||
| 167 | * Send mail to tutor |
||
| 168 | * |
||
| 169 | * @return void |
||
| 170 | */ |
||
| 171 | public function actionMail($id) |
||
| 172 | { |
||
| 173 | try { |
||
| 174 | $tutors = $this->getModel()->getTutor($id); |
||
| 175 | $recipients = $this->parseTutorEmail($tutors); |
||
| 176 | |||
| 177 | $this->getEmailer()->tutor($recipients, $tutors->guid, 'block'); |
||
| 178 | |||
| 179 | Debugger::log('Sending email to block tutor successfull, result: ' . json_encode($recipients) . ', ' . $tutors->guid, Debugger::INFO); |
||
| 180 | $this->flashMessage('Email lektorovi byl odeslán..', 'ok'); |
||
| 181 | } catch(Exception $e) { |
||
| 182 | Debugger::log('Sending email to block tutor failed, result: ' . $e->getMessage(), Debugger::ERROR); |
||
| 183 | $this->flashMessage('Email lektorovi nebyl odeslán, result: ' . $e->getMessage(), 'error'); |
||
| 184 | } |
||
| 185 | |||
| 186 | $this->redirect('Block:edit', $id); |
||
| 187 | } |
||
| 188 | |||
| 189 | /** |
||
| 190 | * Prepare data for editing |
||
| 191 | * |
||
| 192 | * @param int $id of Block |
||
| 193 | * @return void |
||
| 194 | */ |
||
| 195 | public function renderEdit($id) |
||
| 196 | { |
||
| 197 | $template = $this->getTemplate(); |
||
| 198 | |||
| 199 | $template->heading = 'úprava bloku'; |
||
| 200 | $template->page = $this->getHttpRequest()->getQuery('page'); |
||
| 201 | $template->error_name = ""; |
||
| 202 | $template->error_description = ""; |
||
| 203 | $template->error_tutor = ""; |
||
| 204 | $template->error_email = ""; |
||
| 205 | $template->error_material = ""; |
||
| 206 | |||
| 207 | $this->blockId = $id; |
||
| 208 | $block = $this->getModel()->find($id); |
||
| 209 | $template->block = $block; |
||
| 210 | $template->id = $id; |
||
| 211 | |||
| 212 | $template->day_roll = $this->renderHtmlSelectBox('day', $this->days, $block->day, 'width:172px;'); |
||
| 213 | $template->hour_roll = $this->renderHtmlSelectBox('start_hour', $this->hours, $block->from->format('%H')); |
||
| 214 | $template->minute_roll = $this->renderHtmlSelectBox('start_minute', $this->minutes, $block->from->format('%I')); |
||
| 215 | $template->end_hour_roll = $this->renderHtmlSelectBox('end_hour', $this->hours, $block->to->format('%H')); |
||
| 216 | $template->end_minute_roll = $this->renderHtmlSelectBox('end_minute', $this->minutes, $block->to->format('%I')); |
||
| 217 | // is program block check box |
||
| 218 | $template->program_checkbox = $this->renderHtmlCheckBox('program', 1, $block->program); |
||
| 219 | // display programs in block check box |
||
| 220 | $template->display_progs_checkbox = $this->renderHtmlCheckBox('display_progs', 0, $block->display_progs); |
||
| 221 | $template->selectedCategory = $block->category; |
||
| 222 | } |
||
| 223 | |||
| 224 | /** |
||
| 225 | * @return void |
||
| 226 | */ |
||
| 227 | View Code Duplication | public function renderListing() |
|
| 228 | { |
||
| 229 | $model = $this->getModel(); |
||
| 230 | $template = $this->getTemplate(); |
||
| 231 | |||
| 232 | $template->blocks = $model->all(); |
||
| 233 | $template->mid = $this->meetingId; |
||
| 234 | $template->heading = $this->heading; |
||
| 235 | } |
||
| 236 | |||
| 237 | /** |
||
| 238 | * @return Emailer |
||
| 239 | */ |
||
| 240 | protected function getEmailer() |
||
| 244 | |||
| 245 | /** |
||
| 246 | * @param Emailer $emailer |
||
| 247 | * @return $this |
||
| 248 | */ |
||
| 249 | protected function setEmailer(Emailer $emailer) |
||
| 254 | |||
| 255 | /** |
||
| 256 | * Render select box |
||
| 257 | * |
||
| 258 | * @param string name |
||
| 259 | * @param array content of slect box |
||
| 260 | * @param var variable that match selected option |
||
| 261 | * @param string inline styling |
||
| 262 | * @return string html of select box |
||
| 263 | */ |
||
| 264 | private function renderHtmlSelectBox($name, $select_content, $selected_option, $inline_style = NULL) |
||
| 284 | |||
| 285 | /** |
||
| 286 | * @param date $from |
||
| 287 | * @param date $to |
||
| 288 | * @return Exception |
||
| 289 | */ |
||
| 290 | private function guardToGreaterThanFrom($from, $to) |
||
| 291 | { |
||
| 292 | if($from > $to) { |
||
| 293 | throw new Exception('Starting time is greater then finishing time.'); |
||
| 294 | } |
||
| 295 | } |
||
| 296 | |||
| 297 | |||
| 298 | /** |
||
| 299 | * @return MeetingModel |
||
| 300 | */ |
||
| 301 | public function getMeetingModel(): MeetingModel |
||
| 305 | |||
| 306 | /** |
||
| 307 | * @param MeetingModel $meetingModel |
||
| 308 | * |
||
| 309 | * @return self |
||
| 310 | */ |
||
| 311 | public function setMeetingModel(MeetingModel $meetingModel): self |
||
| 317 | |||
| 318 | } |
||
| 319 |
It seems like the type of the argument is not accepted by the function/method which you are calling.
In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.
We suggest to add an explicit type cast like in the following example: