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 XsitemapUtility 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 XsitemapUtility, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
31 | class XsitemapUtility extends XoopsObject |
||
|
|||
32 | { |
||
33 | /** |
||
34 | * |
||
35 | * Verifies XOOPS version meets minimum requirements for this module |
||
36 | * @static |
||
37 | * @param XoopsModule $module |
||
38 | * |
||
39 | * @return bool true if meets requirements, false if not |
||
40 | */ |
||
41 | public static function checkVerXoops(XoopsModule $module) |
||
74 | |||
75 | /** |
||
76 | * |
||
77 | * Verifies PHP version meets minimum requirements for this module |
||
78 | * @static |
||
79 | * @param XoopsModule $module |
||
80 | * |
||
81 | * @return bool true if meets requirements, false if not |
||
82 | */ |
||
83 | public static function checkVerPhp(XoopsModule $module) |
||
99 | |||
100 | /** |
||
101 | * |
||
102 | * Remove files and (sub)directories |
||
103 | * |
||
104 | * @param string $src source directory to delete |
||
105 | * |
||
106 | * @uses \Xmf\Module\Helper::getHelper() |
||
107 | * @uses \Xmf\Module\Helper::isUserAdmin() |
||
108 | * |
||
109 | * @return bool true on success |
||
110 | */ |
||
111 | public static function deleteDirectory($src) |
||
148 | |||
149 | /** |
||
150 | * |
||
151 | * Recursively remove directory |
||
152 | * |
||
153 | * @todo currently won't remove directories with hidden files, should it? |
||
154 | * |
||
155 | * @param string $src directory to remove (delete) |
||
156 | * |
||
157 | * @return bool true on success |
||
158 | */ |
||
159 | public static function rrmdir($src) |
||
190 | |||
191 | /** |
||
192 | * Recursively move files from one directory to another |
||
193 | * |
||
194 | * @param string $src - Source of files being moved |
||
195 | * @param string $dest - Destination of files being moved |
||
196 | * |
||
197 | * @return bool true on success |
||
198 | */ |
||
199 | public static function rmove($src, $dest) |
||
230 | |||
231 | /** |
||
232 | * Recursively copy directories and files from one directory to another |
||
233 | * |
||
234 | * @param string $src - Source of files being moved |
||
235 | * @param string $dest - Destination of files being moved |
||
236 | * |
||
237 | * @uses \Xmf\Module\Helper::getHelper() |
||
238 | * @uses \Xmf\Module\Helper::isUserAdmin() |
||
239 | * |
||
240 | * @return bool true on success |
||
241 | */ |
||
242 | public static function rcopy($src, $dest) |
||
270 | } |
||
271 |
You can fix this by adding a namespace to your class:
When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries.