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:
Complex classes like NewbbTopicRenderer often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.
Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.
While breaking up the class, it is a good idea to analyze how other classes use NewbbTopicRenderer, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
22 | class NewbbTopicRenderer |
||
23 | { |
||
24 | /** |
||
25 | * reference to an object handler |
||
26 | */ |
||
27 | public $handler; |
||
28 | |||
29 | /** |
||
30 | * reference to moduleConfig |
||
31 | */ |
||
32 | public $config; |
||
33 | |||
34 | /** |
||
35 | * Requested page |
||
36 | */ |
||
37 | public $page = 'list.topic.php'; |
||
38 | |||
39 | /** |
||
40 | * query variables |
||
41 | */ |
||
42 | public $args = ['forum', 'uid', 'lastposter', 'type', 'status', 'mode', 'sort', 'order', 'start', 'since']; |
||
43 | public $vars = []; |
||
44 | |||
45 | /** |
||
46 | * For multiple forums |
||
47 | */ |
||
48 | public $is_multiple = false; |
||
49 | |||
50 | /** |
||
51 | * force to parse vars (run against static vars) irmtfan |
||
52 | */ |
||
53 | public $force = false; |
||
54 | |||
55 | /** |
||
56 | * Vistitor's level: 0 - anonymous; 1 - user; 2 - moderator or admin |
||
57 | */ |
||
58 | public $userlevel = 0; |
||
59 | |||
60 | /** |
||
61 | * Current user has no access to current page |
||
62 | */ |
||
63 | public $noperm = false; |
||
64 | |||
65 | /** |
||
66 | * |
||
67 | */ |
||
68 | public $query = []; |
||
69 | |||
70 | /** |
||
71 | * Constructor |
||
72 | */ |
||
73 | // public function NewbbTopicRenderer() |
||
74 | public function __construct() |
||
78 | |||
79 | /** |
||
80 | * Access the only instance of this class |
||
81 | * @return NewbbTopicRenderer |
||
82 | */ |
||
83 | public static function getInstance() |
||
92 | |||
93 | public function init() |
||
98 | |||
99 | /** |
||
100 | * @param $var |
||
101 | * @param $val |
||
102 | * @return array|int|string |
||
103 | */ |
||
104 | public function setVar($var, $val) |
||
146 | |||
147 | /** |
||
148 | * @param array $vars |
||
149 | */ |
||
150 | public function setVars(array $vars = []) |
||
162 | |||
163 | /** |
||
164 | * @param null $status |
||
165 | */ |
||
166 | public function myParseStatus($status = null) |
||
167 | { |
||
168 | switch ($status) { |
||
169 | case 'digest': |
||
170 | $this->query['where'][] = 't.topic_digest = 1'; |
||
171 | break; |
||
172 | |||
173 | case 'undigest': |
||
174 | $this->query['where'][] = 't.topic_digest = 0'; |
||
175 | break; |
||
176 | |||
177 | case 'sticky': |
||
178 | $this->query['where'][] = 't.topic_sticky = 1'; |
||
179 | break; |
||
180 | |||
181 | case 'unsticky': |
||
182 | $this->query['where'][] = 't.topic_sticky = 0'; |
||
183 | break; |
||
184 | |||
185 | case 'lock': |
||
186 | $this->query['where'][] = 't.topic_status = 1'; |
||
187 | break; |
||
188 | |||
189 | case 'unlock': |
||
190 | $this->query['where'][] = 't.topic_status = 0'; |
||
191 | break; |
||
192 | |||
193 | case 'poll': |
||
194 | $this->query['where'][] = 't.topic_haspoll = 1'; |
||
195 | break; |
||
196 | |||
197 | case 'unpoll': |
||
198 | $this->query['where'][] = 't.topic_haspoll = 0'; |
||
199 | break; |
||
200 | |||
201 | case 'voted': |
||
202 | $this->query['where'][] = 't.votes > 0'; |
||
203 | break; |
||
204 | |||
205 | case 'unvoted': |
||
206 | $this->query['where'][] = 't.votes < 1'; |
||
207 | break; |
||
208 | |||
209 | case 'replied': |
||
210 | $this->query['where'][] = 't.topic_replies > 0'; |
||
211 | break; |
||
212 | |||
213 | case 'unreplied': |
||
214 | $this->query['where'][] = 't.topic_replies < 1'; |
||
215 | break; |
||
216 | |||
217 | case 'viewed': |
||
218 | $this->query['where'][] = 't.topic_views > 0'; |
||
219 | break; |
||
220 | |||
221 | case 'unviewed': |
||
222 | $this->query['where'][] = 't.topic_views < 1'; |
||
223 | break; |
||
224 | |||
225 | case 'read': |
||
226 | // Skip |
||
227 | if (empty($this->config['read_mode'])) { |
||
228 | // Use database |
||
229 | View Code Duplication | } elseif ($this->config['read_mode'] == 2) { |
|
230 | // START irmtfan use read_uid to find the unread posts when the user is logged in |
||
231 | $read_uid = is_object($GLOBALS['xoopsUser']) ? $GLOBALS['xoopsUser']->getVar('uid') : 0; |
||
232 | if (!empty($read_uid)) { |
||
233 | $this->query['join'][] = 'LEFT JOIN ' . $this->handler->db->prefix('newbb_reads_topic') . ' AS r ON r.read_item = t.topic_id AND r.uid = ' . $read_uid . ' '; |
||
234 | $this->query['where'][] = 'r.post_id = t.topic_last_post_id'; |
||
235 | } else { |
||
236 | } |
||
237 | // END irmtfan change criteria to get from uid p.uid = last post submit user id |
||
238 | // User cookie |
||
239 | } elseif ($this->config['read_mode'] == 1) { |
||
240 | // START irmtfan fix read_mode = 1 bugs - for all users (member and anon) |
||
241 | $startdate = !empty($this->vars['since']) ? (time() - newbb_getSinceTime($this->vars['since'])) : 0; |
||
242 | if ($lastvisit = max($GLOBALS['last_visit'], $startdate)) { |
||
243 | $readmode1query = ''; |
||
244 | if ($lastvisit > $startdate) { |
||
245 | $readmode1query = 'p.post_time < ' . $lastvisit; |
||
246 | } |
||
247 | $topics = []; |
||
248 | $topic_lastread = newbb_getcookie('LT', true); |
||
249 | View Code Duplication | if (count($topic_lastread) > 0) { |
|
250 | foreach ($topic_lastread as $id => $time) { |
||
251 | if ($time > $lastvisit) { |
||
252 | $topics[] = $id; |
||
253 | } |
||
254 | } |
||
255 | } |
||
256 | if (count($topics) > 0) { |
||
257 | $topicquery = ' t.topic_id IN (' . implode(',', $topics) . ')'; |
||
258 | // because it should be OR |
||
259 | $readmode1query = !empty($readmode1query) ? '(' . $readmode1query . ' OR ' . $topicquery . ')' : $topicquery; |
||
260 | } |
||
261 | $this->query['where'][] = $readmode1query; |
||
262 | } |
||
263 | // END irmtfan fix read_mode = 1 bugs - for all users (member and anon) |
||
264 | } |
||
265 | break; |
||
266 | |||
267 | case 'unread': |
||
268 | // Skip |
||
269 | if (empty($this->config['read_mode'])) { |
||
270 | // Use database |
||
271 | View Code Duplication | } elseif ($this->config['read_mode'] == 2) { |
|
272 | // START irmtfan use read_uid to find the unread posts when the user is logged in |
||
273 | $read_uid = is_object($GLOBALS['xoopsUser']) ? $GLOBALS['xoopsUser']->getVar('uid') : 0; |
||
274 | if (!empty($read_uid)) { |
||
275 | $this->query['join'][] = 'LEFT JOIN ' . $this->handler->db->prefix('newbb_reads_topic') . ' AS r ON r.read_item = t.topic_id AND r.uid = ' . $read_uid . ' '; |
||
276 | $this->query['where'][] = '(r.read_id IS NULL OR r.post_id < t.topic_last_post_id)'; |
||
277 | } else { |
||
278 | } |
||
279 | // END irmtfan change criteria to get from uid p.uid = last post submit user id |
||
280 | // User cookie |
||
281 | } elseif ($this->config['read_mode'] == 1) { |
||
282 | // START irmtfan fix read_mode = 1 bugs - for all users (member and anon) |
||
283 | $startdate = !empty($this->vars['since']) ? (time() - newbb_getSinceTime($this->vars['since'])) : 0; |
||
284 | if ($lastvisit = max($GLOBALS['last_visit'], $startdate)) { |
||
285 | if ($lastvisit > $startdate) { |
||
286 | $this->query['where'][] = 'p.post_time > ' . $lastvisit; |
||
287 | } |
||
288 | $topics = []; |
||
289 | $topic_lastread = newbb_getcookie('LT', true); |
||
290 | View Code Duplication | if (count($topic_lastread) > 0) { |
|
291 | foreach ($topic_lastread as $id => $time) { |
||
292 | if ($time > $lastvisit) { |
||
293 | $topics[] = $id; |
||
294 | } |
||
295 | } |
||
296 | } |
||
297 | if (count($topics) > 0) { |
||
298 | $this->query['where'][] = ' t.topic_id NOT IN (' . implode(',', $topics) . ')'; |
||
299 | } |
||
300 | } |
||
301 | // END irmtfan fix read_mode = 1 bugs - for all users (member and anon) |
||
302 | } |
||
303 | break; |
||
304 | |||
305 | View Code Duplication | case 'pending': |
|
306 | if ($this->userlevel < 2) { |
||
307 | $this->noperm = true; |
||
308 | } else { |
||
309 | $this->query['where'][] = 't.approved = 0'; |
||
310 | } |
||
311 | break; |
||
312 | |||
313 | View Code Duplication | case 'deleted': |
|
314 | if ($this->userlevel < 2) { |
||
315 | $this->noperm = true; |
||
316 | } else { |
||
317 | $this->query['where'][] = 't.approved = -1'; |
||
318 | } |
||
319 | break; |
||
320 | |||
321 | case 'all': // For viewall.php; do not display sticky topics at first |
||
322 | case 'active': // same as 'all' |
||
323 | $this->query['where'][] = 't.approved = 1'; |
||
324 | break; |
||
325 | |||
326 | default: // irmtfan do nothing |
||
327 | break; |
||
328 | } |
||
329 | } |
||
330 | |||
331 | /** |
||
332 | * @param $var |
||
333 | * @param $val |
||
334 | */ |
||
335 | public function parseVar($var, $val) |
||
336 | { |
||
337 | switch ($var) { |
||
338 | case 'forum': |
||
339 | /** @var \NewbbForumHandler $forumHandler */ |
||
340 | $forumHandler = xoops_getModuleHandler('forum', 'newbb'); |
||
341 | // START irmtfan - get forum Ids by values. parse positive values to forum IDs and negative values to category IDs. value=0 => all valid forums |
||
342 | // Get accessible forums |
||
343 | $accessForums = $forumHandler->getIdsByValues(array_map('intval', @explode('|', $val))); |
||
344 | // Filter specified forums if any |
||
345 | //if (!empty($val) && $_forums = @explode('|', $val)) { |
||
346 | //$accessForums = array_intersect($accessForums, array_map('intval', $_forums)); |
||
347 | //} |
||
348 | $this->vars['forum'] = $this->setVar('forum', $accessForums); |
||
349 | // END irmtfan - get forum Ids by values. parse positive values to forum IDs and negative values to category IDs. value=0 => all valid forums |
||
350 | |||
351 | if (empty($accessForums)) { |
||
352 | $this->noperm = true; |
||
353 | // irmtfan - it just return return the forum_id only when the forum_id is the first allowed forum - no need for this code implode is enough removed. |
||
354 | //} elseif (count($accessForums) === 1) { |
||
355 | //$this->query["where"][] = "t.forum_id = " . $accessForums[0]; |
||
356 | } else { |
||
357 | $this->query['where'][] = 't.forum_id IN ( ' . implode(', ', $accessForums) . ' )'; |
||
358 | } |
||
359 | break; |
||
360 | |||
361 | View Code Duplication | case 'uid': // irmtfan add multi topic poster |
|
362 | if ($val !== -1) { |
||
363 | $val = implode(',', array_map('intval', explode(',', $val))); |
||
364 | $this->query['where'][] = 't.topic_poster IN ( ' . $val . ' )'; |
||
365 | } |
||
366 | break; |
||
367 | View Code Duplication | case 'lastposter': // irmtfan add multi lastposter |
|
368 | if ($val !== -1) { |
||
369 | $val = implode(',', array_map('intval', explode(',', $val))); |
||
370 | $this->query['where'][] = 'p.uid IN ( ' . $val . ' )'; |
||
371 | } |
||
372 | break; |
||
373 | |||
374 | case 'since': |
||
375 | if (!empty($val)) { |
||
376 | // START irmtfan if unread && read_mode = 1 and last_visit > startdate do not add where query | to accept multiple status |
||
377 | $startdate = time() - newbb_getSinceTime($val); |
||
378 | if (in_array('unread', explode(',', $this->vars['status'], true)) && $this->config['read_mode'] == 1 |
||
379 | && $GLOBALS['last_visit'] > $startdate) { |
||
380 | break; |
||
381 | } |
||
382 | // irmtfan digest_time | to accept multiple status |
||
383 | if (in_array('digest', explode(',', $this->vars['status'], true))) { |
||
384 | $this->query['where'][] = 't.digest_time > ' . $startdate; |
||
385 | } |
||
386 | // irmtfan - should be >= instead of = |
||
387 | $this->query['where'][] = 'p.post_time >= ' . $startdate; |
||
388 | // END irmtfan if unread && read_mode = 1 and last_visit > startdate do not add where query |
||
389 | } |
||
390 | break; |
||
391 | |||
392 | case 'type': |
||
393 | if (!empty($val)) { |
||
394 | $this->query['where'][] = 't.type_id = ' . $val; |
||
395 | } |
||
396 | break; |
||
397 | |||
398 | case 'status': |
||
399 | // START irmtfan to accept multiple status |
||
400 | $val = explode(',', $val); |
||
401 | // irmtfan - add 'all' to always parse t.approved = 1 |
||
402 | if (count(array_intersect($val, ['all', 'active', 'pending', 'deleted'])) === 0) { |
||
403 | $val[] = 'all'; |
||
404 | } |
||
405 | foreach ($val as $key => $status) { |
||
406 | $this->myParseStatus($status); |
||
407 | } |
||
408 | // END irmtfan to accept multiple status |
||
409 | break; |
||
410 | |||
411 | case 'sort': |
||
412 | if ($sort = $this->getSort($val, 'sort')) { |
||
413 | $this->query['sort'][] = $sort . (empty($this->vars['order']) ? ' DESC' : ' ASC'); |
||
414 | } else { // irmtfan if sort is not in the list |
||
415 | $this->query['sort'][] = 't.topic_last_post_id' . (empty($this->vars['order']) ? ' DESC' : ' ASC'); |
||
416 | } |
||
417 | break; |
||
418 | |||
419 | default: |
||
420 | break; |
||
421 | } |
||
422 | } |
||
423 | |||
424 | /** |
||
425 | * @return bool |
||
426 | */ |
||
427 | public function parseVars() |
||
428 | { |
||
429 | static $parsed; |
||
430 | // irmtfan - force to parse vars (run against static vars) |
||
431 | if (isset($parsed) && !$this->force) { |
||
432 | return true; |
||
433 | } |
||
434 | |||
435 | if (!isset($this->vars['forum'])) { |
||
436 | $this->vars['forum'] = null; |
||
437 | } |
||
438 | //irmtfan parse status for rendering topic correctly - if empty($_GET(status)) it will show all topics include deleted and pendings. 'all' instead of all |
||
439 | if (!isset($this->vars['status'])) { |
||
440 | $this->vars['status'] = 'all'; |
||
441 | } |
||
442 | // irmtfan if sort is not set or is empty get a default sort- if empty($_GET(sort)) | if sort=null eg: /list.topic.php?sort= |
||
443 | if (empty($this->vars['sort'])) { |
||
444 | $this->vars['sort'] = 'lastpost'; |
||
445 | } // use lastpost instead of sticky |
||
446 | |||
447 | foreach ($this->vars as $var => $val) { |
||
448 | $this->parseVar($var, $val); |
||
449 | if (empty($val)) { |
||
450 | unset($this->vars[$var]); |
||
451 | } |
||
452 | } |
||
453 | $parsed = true; |
||
454 | |||
455 | return true; |
||
456 | } |
||
457 | |||
458 | /** |
||
459 | * @param null $header |
||
460 | * @param null $var |
||
461 | * @return array|null |
||
462 | */ |
||
463 | public function getSort($header = null, $var = null) |
||
464 | { |
||
465 | $headers = [ |
||
466 | 'topic' => [ |
||
467 | 'title' => _MD_NEWBB_TOPICS, |
||
468 | 'sort' => 't.topic_title' |
||
469 | ], |
||
470 | 'forum' => [ |
||
471 | 'title' => _MD_NEWBB_FORUM, |
||
472 | 'sort' => 't.forum_id' |
||
473 | ], |
||
474 | 'poster' => [ |
||
475 | 'title' => _MD_NEWBB_TOPICPOSTER, /*irmtfan _MD_NEWBB_POSTER to _MD_NEWBB_TOPICPOSTER*/ |
||
476 | 'sort' => 't.topic_poster' |
||
477 | ], |
||
478 | 'replies' => [ |
||
479 | 'title' => _MD_NEWBB_REPLIES, |
||
480 | 'sort' => 't.topic_replies' |
||
481 | ], |
||
482 | 'views' => [ |
||
483 | 'title' => _MD_NEWBB_VIEWS, |
||
484 | 'sort' => 't.topic_views' |
||
485 | ], |
||
486 | 'lastpost' => [ // irmtfan show topic_page_jump_icon smarty |
||
487 | 'title' => _MD_NEWBB_LASTPOST, |
||
488 | /*irmtfan _MD_NEWBB_DATE to _MD_NEWBB_LASTPOSTTIME again change to _MD_LASTPOST*/ |
||
489 | 'sort' => 't.topic_last_post_id' |
||
490 | ], |
||
491 | // START irmtfan add more sorts |
||
492 | 'lastposttime' => [ // irmtfan same as lastpost |
||
493 | 'title' => _MD_NEWBB_LASTPOSTTIME, |
||
494 | 'sort' => 't.topic_last_post_id' |
||
495 | ], |
||
496 | 'lastposter' => [ // irmtfan |
||
497 | 'title' => _MD_NEWBB_POSTER, |
||
498 | 'sort' => 'p.uid',// poster uid |
||
499 | ], |
||
500 | 'lastpostmsgicon' => [ // irmtfan |
||
501 | 'title' => _MD_NEWBB_MESSAGEICON, |
||
502 | 'sort' => 'p.icon',// post message icon |
||
503 | ], |
||
504 | 'ratings' => [ |
||
505 | 'title' => _MD_NEWBB_RATINGS, |
||
506 | 'sort' => 't.rating', // irmtfan t.topic_rating to t.rating |
||
507 | ], |
||
508 | 'votes' => [ |
||
509 | 'title' => _MD_NEWBB_VOTES, |
||
510 | 'sort' => 't.votes' |
||
511 | ], |
||
512 | 'publish' => [ |
||
513 | 'title' => _MD_NEWBB_TOPICTIME, |
||
514 | 'sort' => 't.topic_id' |
||
515 | ], |
||
516 | 'digest' => [ |
||
517 | 'title' => _MD_NEWBB_DIGEST, |
||
518 | 'sort' => 't.digest_time' |
||
519 | ], |
||
520 | 'sticky' => [ |
||
521 | 'title' => _MD_NEWBB_STICKY, |
||
522 | 'sort' => 't.topic_sticky' |
||
523 | ], |
||
524 | 'lock' => [ |
||
525 | 'title' => _MD_NEWBB_LOCK, |
||
526 | 'sort' => 't.topic_status' |
||
527 | ], |
||
528 | 'poll' => [ |
||
529 | 'title' => _MD_NEWBB_POLL_POLL, |
||
530 | 'sort' => 't.poll_id' |
||
531 | ] |
||
532 | ]; |
||
533 | $types = $this->getTypes(); |
||
534 | if (!empty($types)) { |
||
535 | $headers['type'] = [ |
||
536 | 'title' => _MD_NEWBB_TYPE, |
||
537 | 'sort' => 't.type_id' |
||
538 | ]; |
||
539 | } |
||
540 | if ($this->userlevel == 2) { |
||
541 | $headers['approve'] = [ |
||
542 | 'title' => _MD_NEWBB_APPROVE, |
||
543 | 'sort' => 't.approved' |
||
544 | ]; |
||
545 | } |
||
546 | // END irmtfan add more sorts |
||
547 | if (empty($header) && empty($var)) { |
||
548 | return $headers; |
||
549 | } |
||
550 | if (!empty($var) && !empty($header)) { |
||
551 | return @$headers[$header][$var]; |
||
552 | } |
||
553 | if (empty($var)) { |
||
554 | return @$headers[$header]; |
||
555 | } |
||
556 | $ret = null; |
||
557 | foreach (array_keys($headers) as $key) { |
||
558 | $ret[$key] = @$headers[$key][$var]; |
||
559 | } |
||
560 | |||
561 | return $ret; |
||
562 | } |
||
563 | |||
564 | // START irmtfan add Display topic headers function |
||
565 | |||
566 | /** |
||
567 | * @param null $header |
||
568 | * @return array |
||
569 | */ |
||
570 | public function getHeader($header = null) |
||
582 | |||
583 | // END irmtfan add Display topic headers function |
||
584 | |||
585 | /** |
||
586 | * @param null $type |
||
587 | * @param null $status |
||
588 | * @return array |
||
589 | */ |
||
590 | public function getStatus($type = null, $status = null) |
||
625 | |||
626 | /** |
||
627 | * @param Smarty $xoopsTpl |
||
628 | */ |
||
629 | public function buildSelection(Smarty $xoopsTpl) |
||
661 | |||
662 | /** |
||
663 | * @param Smarty $xoopsTpl |
||
664 | */ |
||
665 | public function buildSearch(Smarty $xoopsTpl) |
||
674 | |||
675 | /** |
||
676 | * @param Smarty $xoopsTpl |
||
677 | */ |
||
678 | public function buildHeaders(Smarty $xoopsTpl) |
||
702 | |||
703 | /** |
||
704 | * @param Smarty $xoopsTpl |
||
705 | */ |
||
706 | public function buildFilters(Smarty $xoopsTpl) |
||
726 | |||
727 | /** |
||
728 | * @param null $type_id |
||
729 | * @return mixed |
||
730 | */ |
||
731 | public function getTypes($type_id = null) |
||
746 | |||
747 | /** |
||
748 | * @param Smarty $xoopsTpl |
||
749 | * @return bool |
||
750 | */ |
||
751 | public function buildTypes(Smarty $xoopsTpl) |
||
773 | |||
774 | /** |
||
775 | * @param Smarty $xoopsTpl |
||
776 | * @return bool |
||
777 | */ |
||
778 | public function buildCurrent(Smarty $xoopsTpl) |
||
796 | |||
797 | /** |
||
798 | * @param Smarty $xoopsTpl |
||
799 | */ |
||
800 | public function buildPagenav(Smarty $xoopsTpl) |
||
828 | |||
829 | /** |
||
830 | * @return int |
||
831 | */ |
||
832 | public function getCount() |
||
860 | |||
861 | /** |
||
862 | * @param Smarty $xoopsTpl |
||
863 | * @return array|void |
||
864 | */ |
||
865 | public function renderTopics(Smarty $xoopsTpl = null) |
||
1129 | |||
1130 | // START irmtfan to create an array from selected keys of an array |
||
1131 | |||
1132 | /** |
||
1133 | * @param $array |
||
1134 | * @param null $keys |
||
1135 | * @return array |
||
1136 | */ |
||
1137 | public function getFromKeys($array, $keys = null) |
||
1151 | // END irmtfan to create an array from selected keys of an array |
||
1152 | } |
||
1153 |
Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.
The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.
This check looks for comments that seem to be mostly valid code and reports them.