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 qte 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 qte, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
15 | class qte |
||
16 | { |
||
17 | const KEEP = -2; |
||
18 | const REMOVE = -1; |
||
19 | |||
20 | /** @var \phpbb\request\request */ |
||
21 | protected $request; |
||
22 | |||
23 | /** @var \phpbb\cache\driver\driver_interface */ |
||
24 | protected $cache; |
||
25 | |||
26 | /** @var \phpbb\db\driver\driver_interface */ |
||
27 | protected $db; |
||
28 | |||
29 | /** @var \phpbb\template\template */ |
||
30 | protected $template; |
||
31 | |||
32 | /** @var \phpbb\user */ |
||
33 | protected $user; |
||
34 | |||
35 | /** @var \phpbb\log\log */ |
||
36 | protected $log; |
||
37 | |||
38 | /** @var \phpbb\auth\auth */ |
||
39 | protected $auth; |
||
40 | |||
41 | /** @var string */ |
||
42 | protected $root_path; |
||
43 | |||
44 | /** @var string */ |
||
45 | protected $php_ext; |
||
46 | |||
47 | /** @var string */ |
||
48 | protected $table_prefix; |
||
49 | |||
50 | /** @var array */ |
||
51 | private $_attr; |
||
52 | |||
53 | /** @var array */ |
||
54 | private $_name = array(); |
||
55 | |||
56 | /** |
||
57 | * Constructor |
||
58 | * |
||
59 | * @param \phpbb\request\request $request Request object |
||
60 | * @param \phpbb\cache\driver\driver_interface $cache Cache object |
||
61 | * @param \phpbb\db\driver\driver_interface $db Database object |
||
62 | * @param \phpbb\template\template $template Template object |
||
63 | * @param \phpbb\user $user User object |
||
64 | * @param \phpbb\log\log $log Log object |
||
65 | * @param \phpbb\auth\auth $auth Auth object |
||
66 | * @param string $root_path phpBB root path |
||
67 | * @param string $php_ext phpEx |
||
68 | * @param string $table_prefix Prefix tables |
||
69 | */ |
||
70 | public function __construct(\phpbb\request\request $request, \phpbb\cache\driver\driver_interface $cache, \phpbb\db\driver\driver_interface $db, \phpbb\template\template $template, \phpbb\user $user, \phpbb\log\log $log, \phpbb\auth\auth $auth, $root_path, $php_ext, $table_prefix) |
||
87 | |||
88 | /** |
||
89 | * Get topic attributes username |
||
90 | * |
||
91 | * @param array $topic_list Topic ids |
||
92 | * |
||
93 | * @return null |
||
94 | */ |
||
95 | public function get_users_by_topic_id($topic_list) |
||
96 | { |
||
97 | if (!empty($topic_list)) |
||
98 | { |
||
99 | $sql = 'SELECT u.user_id, u.username, u.user_colour |
||
100 | FROM ' . USERS_TABLE . ' u |
||
101 | LEFT JOIN ' . TOPICS_TABLE . ' t ON (u.user_id = t.topic_attr_user) |
||
102 | WHERE ' . $this->db->sql_in_set('t.topic_id', array_map('intval', $topic_list)) . ' |
||
103 | AND t.topic_attr_user <> ' . ANONYMOUS; |
||
104 | $result = $this->db->sql_query($sql); |
||
105 | |||
106 | View Code Duplication | while ($row = $this->db->sql_fetchrow($result)) |
|
107 | { |
||
108 | $this->_name[$row['user_id']] = array( |
||
109 | 'user_id' => (int) $row['user_id'], |
||
110 | 'username' => $row['username'], |
||
111 | 'user_colour' => $row['user_colour'], |
||
112 | ); |
||
113 | } |
||
114 | $this->db->sql_freeresult(); |
||
115 | } |
||
116 | } |
||
117 | |||
118 | /** |
||
119 | * Get attribute name |
||
120 | * |
||
121 | * @param int $attr_id The attribute id |
||
122 | * |
||
123 | * @return string |
||
124 | */ |
||
125 | public function get_attr_name_by_id($attr_id) |
||
129 | |||
130 | /** |
||
131 | * Get attribute author |
||
132 | * |
||
133 | * @param int $user_id User id |
||
134 | * |
||
135 | * @return string |
||
136 | */ |
||
137 | public function get_users_by_user_id($user_id) |
||
157 | |||
158 | /** |
||
159 | * Generate a list of attributes based on permissions |
||
160 | * |
||
161 | * @param int $forum_id Forum id |
||
162 | * @param int $author_id Topic author id |
||
163 | * @param int $attribute_id Current attribute id |
||
164 | * @param string $viewtopic_url Topic's url |
||
165 | * @param string $mode Post mode |
||
166 | * |
||
167 | * @return null |
||
168 | */ |
||
169 | public function attr_select($forum_id, $author_id = 0, $attribute_id = 0, $viewtopic_url = '', $mode = '') |
||
170 | { |
||
171 | $show_select = false; |
||
172 | $current_time = time(); |
||
173 | $can_edit = (bool) $this->auth->acl_get('m_qte_attr_edit', $forum_id); |
||
174 | $can_remove = (bool) $this->auth->acl_get('m_qte_attr_del', $forum_id); |
||
175 | $is_author = (bool) ($this->user->data['is_registered'] && $this->user->data['user_id'] == $author_id); |
||
176 | |||
177 | if ($can_edit || $is_author || $mode == 'post') |
||
178 | { |
||
179 | foreach ($this->_attr as $attr) |
||
180 | { |
||
181 | if (!$this->auth->acl_get('f_qte_attr_'.$attr['attr_id'], $forum_id)) |
||
182 | { |
||
183 | continue; |
||
184 | } |
||
185 | |||
186 | // show the selector ! |
||
187 | $show_select = true; |
||
188 | |||
189 | // parse the attribute name |
||
190 | $attribute_name = str_replace(array('%mod%', '%date%'), array($this->user->data['username'], $this->user->format_date($current_time, $attr['attr_date'])), $this->user->lang($attr['attr_name'])); |
||
191 | |||
192 | $this->template->assign_block_vars('attributes', array( |
||
193 | 'QTE_ID' => $attr['attr_id'], |
||
194 | 'QTE_NAME' => $attribute_name, |
||
195 | 'QTE_DESC' => $this->user->lang($attr['attr_desc']), |
||
196 | 'QTE_COLOUR' => $this->attr_colour($attr['attr_name'], $attr['attr_colour']), |
||
197 | |||
198 | 'IS_SELECTED' => (!empty($attribute_id) && ($attr['attr_id'] == $attribute_id)), |
||
199 | |||
200 | 'S_QTE_DESC' => !empty($attr['attr_desc']) ? true : false, |
||
201 | 'U_QTE_URL' => !empty($viewtopic_url) ? append_sid($viewtopic_url, array('attr_id' => $attr['attr_id'])) : false, |
||
202 | )); |
||
203 | } |
||
204 | } |
||
205 | |||
206 | $this->template->assign_vars(array( |
||
207 | 'S_QTE_SELECT' => ($show_select || $can_remove && ($attribute_id || !$author_id)), |
||
208 | 'S_QTE_REMOVE' => $can_remove, |
||
209 | 'S_QTE_EMPTY' => (empty($attribute_id)), |
||
210 | 'S_QTE_SELECTED' => ($can_remove && ($attribute_id == self::REMOVE)), |
||
211 | 'S_QTE_KEEP' => !empty($attribute_id) && ($attribute_id == self::KEEP), |
||
212 | |||
213 | 'U_QTE_URL' => !empty($viewtopic_url) ? append_sid($viewtopic_url, array('attr_id' => self::REMOVE)) : false, |
||
214 | )); |
||
215 | } |
||
216 | |||
217 | /** |
||
218 | * Generate a list of all attributes for search page |
||
219 | * |
||
220 | * @return null |
||
221 | */ |
||
222 | public function attr_search() |
||
239 | |||
240 | /** |
||
241 | * Generate a list of attributes for viewforum page |
||
242 | * |
||
243 | * @param int $forum_id Forum id |
||
244 | * @param int $attribute_id Current attribute id |
||
245 | * |
||
246 | * @return null |
||
247 | */ |
||
248 | View Code Duplication | public function attr_sort($forum_id = 0, $attribute_id = 0) |
|
|
|||
249 | { |
||
250 | foreach ($this->_attr as $attr) |
||
251 | { |
||
252 | $forum_allowed = $this->auth->acl_getf('f_qte_attr_'.$attr['attr_id'], true); |
||
253 | |||
254 | if (isset($forum_allowed[$forum_id])) |
||
255 | { |
||
256 | // parse the attribute name |
||
257 | $attribute_name = str_replace(array('%mod%', '%date%'), array($this->user->lang['QTE_KEY_USERNAME'], $this->user->lang['QTE_KEY_DATE']), $this->user->lang($attr['attr_name'])); |
||
258 | |||
259 | $this->template->assign_block_vars('attributes', array( |
||
260 | 'QTE_ID' => $attr['attr_id'], |
||
261 | 'QTE_NAME' => $attribute_name, |
||
262 | 'QTE_DESC' => $this->user->lang($attr['attr_desc']), |
||
263 | 'QTE_COLOUR' => $this->attr_colour($attr['attr_name'], $attr['attr_colour']), |
||
264 | |||
265 | 'IS_SELECTED' => (!empty($attribute_id) && ($attr['attr_id'] == $attribute_id)) ? true : false, |
||
266 | |||
267 | 'S_QTE_DESC' => !empty($attr['attr_desc']) ? true : false, |
||
268 | )); |
||
269 | } |
||
270 | } |
||
271 | } |
||
272 | |||
273 | /** |
||
274 | * Generate a default attribute list for a forum |
||
275 | * |
||
276 | * @param int $forum_id Forum id |
||
277 | * @param int $attribute_id Current attribute id |
||
278 | * |
||
279 | * @return null |
||
280 | */ |
||
281 | View Code Duplication | public function attr_default($forum_id = 0, $attribute_id = 0) |
|
305 | |||
306 | /** |
||
307 | * Generate attribute for topic title |
||
308 | * |
||
309 | * @param int $attribute_id Current attribute id |
||
310 | * @param int $user_id Current attribute user id |
||
311 | * @param int $timestamp Attribute timestamp |
||
312 | * |
||
313 | * @return string Attribute html code |
||
314 | */ |
||
315 | public function attr_display($attribute_id = 0, $user_id = 0, $timestamp = 0) |
||
342 | |||
343 | /** |
||
344 | * Generate attribute for page title |
||
345 | * |
||
346 | * @param int $attribute_id Current attribute id |
||
347 | * @param int $user_id Current attribute user id |
||
348 | * @param int $timestamp Attribute timestamp |
||
349 | * |
||
350 | * @return string attribute html code |
||
351 | */ |
||
352 | public function attr_title($attribute_id = 0, $user_id = 0, $timestamp = 0) |
||
375 | |||
376 | |||
377 | /** |
||
378 | * Change topic attribute |
||
379 | * |
||
380 | * @param int $attribute_id New attribute id |
||
381 | * @param int $topic_id The id of the topic |
||
382 | * @param int $forum_id The id of the forum |
||
383 | * @param int $topic_attribute Current attribute id |
||
384 | * @param int $author_id Topic author id |
||
385 | * @param string $viewtopic_url URL to the topic page |
||
386 | * |
||
387 | * @return null |
||
388 | */ |
||
389 | public function attr_apply($attribute_id = 0, $topic_id = 0, $forum_id = 0, $topic_attribute = 0, $author_id = 0, $viewtopic_url = '') |
||
390 | { |
||
391 | if (empty($topic_id) || empty($forum_id) || empty($attribute_id)) |
||
392 | { |
||
393 | return; |
||
394 | } |
||
395 | |||
396 | $can_edit = $this->auth->acl_get('m_qte_attr_edit', $forum_id) || $this->auth->acl_get('f_qte_attr_'.$attribute_id, $forum_id) && $this->user->data['is_registered'] && $this->user->data['user_id'] == $author_id; |
||
397 | $can_remove = $this->auth->acl_get('m_qte_attr_del', $forum_id); |
||
398 | |||
399 | View Code Duplication | if (!$can_edit && $attribute_id != self::REMOVE || !$can_remove && $attribute_id == self::REMOVE) |
|
400 | { |
||
401 | return; |
||
402 | } |
||
403 | |||
404 | // Default values |
||
405 | $fields = array('topic_attr_id' => 0, 'topic_attr_user' => 0, 'topic_attr_time' => 0); |
||
406 | |||
407 | // time ! |
||
408 | $current_time = time(); |
||
409 | |||
410 | View Code Duplication | if ($attribute_id != self::REMOVE) |
|
411 | { |
||
412 | $fields = array( |
||
413 | 'topic_attr_id' => $attribute_id, |
||
414 | 'topic_attr_user' => $this->user->data['user_id'], |
||
415 | 'topic_attr_time' => $current_time, |
||
416 | ); |
||
417 | } |
||
418 | |||
419 | $sql = 'UPDATE ' . TOPICS_TABLE . ' |
||
420 | SET ' . $this->db->sql_build_array('UPDATE', $fields) . ' |
||
421 | WHERE topic_id = ' . (int) $topic_id; |
||
422 | $this->db->sql_query($sql); |
||
423 | |||
424 | $sql = 'SELECT topic_id |
||
425 | FROM ' . TOPICS_TABLE . ' |
||
426 | WHERE topic_moved_id = ' . (int) $topic_id; |
||
427 | $result = $this->db->sql_query($sql); |
||
428 | $shadow_topic_id = (int) $this->db->sql_fetchfield('topic_id'); |
||
429 | $this->db->sql_freeresult($result); |
||
430 | |||
431 | if (!empty($shadow_topic_id)) |
||
432 | { |
||
433 | $sql = 'UPDATE ' . TOPICS_TABLE . ' |
||
434 | SET ' . $this->db->sql_build_array('UPDATE', $fields) . ' |
||
435 | WHERE topic_id = ' . $shadow_topic_id; |
||
436 | $this->db->sql_query($sql); |
||
437 | } |
||
438 | |||
439 | meta_refresh(2, $viewtopic_url); |
||
440 | |||
441 | $message = $this->user->lang['QTE_ATTRIBUTE_' . ($attribute_id == -1 ? 'REMOVED' : (empty($topic_attribute) ? 'ADDED' : 'UPDATED'))]; |
||
442 | |||
443 | if ($this->request->is_ajax()) |
||
444 | { |
||
445 | $json_response = new \phpbb\json_response; |
||
446 | $json_response->send(array( |
||
447 | 'success' => true, |
||
448 | |||
449 | 'MESSAGE_TITLE' => $this->user->lang['INFORMATION'], |
||
450 | 'MESSAGE_TEXT' => $message, |
||
451 | 'NEW_ATTRIBUTE' => $this->attr_display($attribute_id, $this->user->data['user_id'], $current_time), |
||
452 | )); |
||
453 | } |
||
454 | |||
455 | $message .= '<br /><br />' . $this->user->lang('RETURN_PAGE', '<a href="' . $viewtopic_url . '">', '</a>'); |
||
456 | |||
457 | trigger_error($message); |
||
458 | } |
||
459 | |||
460 | /** |
||
461 | * Change topic attribute in mcp |
||
462 | * |
||
463 | * @param int $attribute_id New attribute id |
||
464 | * @param int $forum_id The id of the forum |
||
465 | * @param array $topic_ids Topics ids |
||
466 | * |
||
467 | * @return null |
||
468 | */ |
||
469 | public function mcp_attr_apply($attribute_id = 0, $forum_id = 0, $topic_ids = array()) |
||
552 | |||
553 | /** |
||
554 | * Getter... |
||
555 | * |
||
556 | * @return array |
||
557 | */ |
||
558 | public function getAttr() |
||
562 | |||
563 | // borrowed from "Categories Hierarchy" : used to check if a image key exists |
||
564 | public function attr_img_key($key, $alt) |
||
568 | |||
569 | /** |
||
570 | * Build class and style attribute |
||
571 | * |
||
572 | * @param string $a_name Attribute name |
||
573 | * @param string $a_colour Attribute color |
||
574 | * @return string html code |
||
575 | */ |
||
576 | public function attr_colour($a_name, $a_colour) |
||
585 | |||
586 | /** |
||
587 | * Get attributes from database |
||
588 | * |
||
589 | * @return null |
||
590 | */ |
||
591 | private function _get_attributes() |
||
619 | } |
||
620 |
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.