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 XoopstubeTree 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 XoopstubeTree, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
33 | class XoopstubeTree |
||
34 | { |
||
35 | public $table; //table with parent-child structure |
||
36 | public $id; //name of unique id for records in table $table |
||
37 | public $pid; // name of parent id used in table $table |
||
38 | public $order; //specifies the order of query results |
||
39 | public $title; // name of a field in table $table which will be used when selection box and paths are generated |
||
40 | public $db; |
||
41 | |||
42 | //constructor of class XoopsTree |
||
43 | //sets the names of table, unique id, and parend id |
||
44 | /** |
||
45 | * @param $tableName |
||
46 | * @param $idName |
||
47 | * @param $pidName |
||
48 | */ |
||
49 | public function __construct($tableName, $idName, $pidName) |
||
56 | |||
57 | // returns an array of first child objects for a given id($selectId) |
||
58 | /** |
||
59 | * @param $selectId |
||
60 | * @param string $order |
||
61 | * |
||
62 | * @return array |
||
63 | */ |
||
64 | public function getFirstChild($selectId, $order = '') |
||
83 | |||
84 | // returns an array of all FIRST child ids of a given id($selectId) |
||
85 | /** |
||
86 | * @param $selectId |
||
87 | * |
||
88 | * @return array |
||
89 | */ |
||
90 | public function getFirstChildId($selectId) |
||
105 | |||
106 | //returns an array of ALL child ids for a given id($selectId) |
||
107 | /** |
||
108 | * @param $selectId |
||
109 | * @param string $order |
||
110 | * @param array $idarray |
||
111 | * |
||
112 | * @return array |
||
113 | */ |
||
114 | View Code Duplication | public function getAllChildId($selectId, $order = '', $idarray = array()) |
|
133 | |||
134 | //returns an array of ALL parent ids for a given id($selectId) |
||
135 | /** |
||
136 | * @param $selectId |
||
137 | * @param string $order |
||
138 | * @param array $idarray |
||
139 | * |
||
140 | * @return array |
||
141 | */ |
||
142 | public function getAllParentId($selectId, $order = '', $idarray = array()) |
||
159 | |||
160 | //generates path from the root id to a given id($selectId) |
||
161 | // the path is delimetered with "/" |
||
162 | /** |
||
163 | * @param $selectId |
||
164 | * @param $title |
||
165 | * @param string $path |
||
166 | * |
||
167 | * @return string |
||
168 | */ |
||
169 | public function getPathFromId($selectId, $title, $path = '') |
||
187 | |||
188 | //makes a nicely ordered selection box |
||
189 | //$preset_id is used to specify a preselected item |
||
190 | //set $none to 1 to add a option with value 0 |
||
191 | /** |
||
192 | * @param $title |
||
193 | * @param string $order |
||
194 | * @param int $preset_id |
||
195 | * @param int $none |
||
196 | * @param string $sel_name |
||
197 | * @param string $onchange |
||
198 | */ |
||
199 | public function makeMySelBox($title, $order = '', $preset_id = 0, $none = 0, $sel_name = '', $onchange = '') |
||
238 | |||
239 | //generates nicely formatted linked path from the root id to a given id |
||
240 | /** |
||
241 | * @param $selectId |
||
242 | * @param $title |
||
243 | * @param $funcURL |
||
244 | * @param string $path |
||
245 | * |
||
246 | * @return string |
||
247 | */ |
||
248 | public function getNicePathFromId($selectId, $title, $funcURL, $path = '') |
||
268 | |||
269 | //generates id path from the root id to a given id |
||
270 | // the path is delimetered with "/" |
||
271 | /** |
||
272 | * @param $selectId |
||
273 | * @param string $path |
||
274 | * |
||
275 | * @return string |
||
276 | */ |
||
277 | public function getIdPathFromId($selectId, $path = '') |
||
293 | |||
294 | /** |
||
295 | * Enter description here... |
||
296 | * |
||
297 | * @param int $selectId |
||
298 | * @param string $order |
||
299 | * @param array $parray |
||
300 | * |
||
301 | * @return array |
||
302 | */ |
||
303 | View Code Duplication | public function getAllChild($selectId = 0, $order = '', $parray = array()) |
|
322 | |||
323 | /** |
||
324 | * Enter description here... |
||
325 | * |
||
326 | * @param int $selectId |
||
327 | * @param string $order |
||
328 | * @param array $parray |
||
329 | * @param string $r_prefix |
||
330 | * |
||
331 | * @return array |
||
332 | */ |
||
333 | public function getChildTreeArray($selectId = 0, $order = '', $parray = array(), $r_prefix = '') |
||
353 | } |
||
354 |
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.