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 ThemeResourceLoader 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 ThemeResourceLoader, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
8 | class ThemeResourceLoader { |
||
9 | |||
10 | /** |
||
11 | * @var ThemeResourceLoader |
||
12 | */ |
||
13 | private static $instance; |
||
14 | |||
15 | protected $base; |
||
16 | |||
17 | /** |
||
18 | * List of template "sets" that contain a test manifest, and have an alias. |
||
19 | * E.g. '$default' |
||
20 | * |
||
21 | * @var ThemeList[] |
||
22 | */ |
||
23 | protected $sets = []; |
||
24 | |||
25 | /** |
||
26 | * @return ThemeResourceLoader |
||
27 | */ |
||
28 | public static function instance() { |
||
31 | |||
32 | /** |
||
33 | * Set instance |
||
34 | * |
||
35 | * @param ThemeResourceLoader $instance |
||
36 | */ |
||
37 | public static function set_instance(ThemeResourceLoader $instance) { |
||
40 | |||
41 | public function __construct($base = null) { |
||
44 | |||
45 | /** |
||
46 | * Add a new theme manifest for a given identifier. E.g. '$default' |
||
47 | * |
||
48 | * @param string $set |
||
49 | * @param ThemeList $manifest |
||
50 | */ |
||
51 | public function addSet($set, ThemeList $manifest) { |
||
54 | |||
55 | /** |
||
56 | * Get a named theme set |
||
57 | * |
||
58 | * @param string $set |
||
59 | * @return ThemeList |
||
60 | */ |
||
61 | public function getSet($set) { |
||
67 | |||
68 | /** |
||
69 | * Given a theme identifier, determine the path from the root directory |
||
70 | * |
||
71 | * The mapping from $identifier to path follows these rules: |
||
72 | * - A simple theme name ('mytheme') which maps to the standard themes dir (/themes/mytheme) |
||
73 | * - A theme path with a leading slash ('/mymodule/themes/mytheme') which maps directly to that path. |
||
74 | * - or a vendored theme path. (vendor/mymodule:mytheme) which maps to the nested 'theme' within |
||
75 | * that module. ('/mymodule/themes/mytheme'). |
||
76 | * - A vendored module with no nested theme (vendor/mymodule) which maps to the root directory |
||
77 | * of that module. ('/mymodule'). |
||
78 | * |
||
79 | * @param string $identifier Theme identifier. |
||
80 | * @return string Path from root, not including leading or trailing forward slash. E.g. themes/mytheme |
||
81 | */ |
||
82 | public function getPath($identifier) { |
||
140 | |||
141 | /** |
||
142 | * Attempts to find possible candidate templates from a set of template |
||
143 | * names from modules, current theme directory and finally the application |
||
144 | * folder. |
||
145 | * |
||
146 | * The template names can be passed in as plain strings, or be in the |
||
147 | * format "type/name", where type is the type of template to search for |
||
148 | * (e.g. Includes, Layout). |
||
149 | * |
||
150 | * @param string|array $template Template name, or template spec in array format with the keys |
||
151 | * 'type' (type string) and 'templates' (template hierarchy in order of precedence). |
||
152 | * If 'templates' is ommitted then any other item in the array will be treated as the template |
||
153 | * list, or list of templates each in the array spec given. |
||
154 | * Templates with an .ss extension will be treated as file paths, and will bypass |
||
155 | * theme-coupled resolution. |
||
156 | * @param array $themes List of themes to use to resolve themes. In most cases |
||
157 | * you should pass in {@see SSViewer::get_themes()} |
||
158 | * @return string Absolute path to resolved template file, or null if not resolved. |
||
159 | * File location will be in the format themes/<theme>/templates/<directories>/<type>/<basename>.ss |
||
160 | * Note that type (e.g. 'Layout') is not the root level directory under 'templates'. |
||
161 | */ |
||
162 | public function findTemplate($template, $themes) { |
||
214 | |||
215 | /** |
||
216 | * Resolve themed CSS path |
||
217 | * |
||
218 | * @param string $name Name of CSS file without extension |
||
219 | * @param array $themes List of themes |
||
220 | * @return string Path to resolved CSS file (relative to base dir) |
||
221 | */ |
||
222 | View Code Duplication | public function findThemedCSS($name, $themes) { |
|
232 | |||
233 | /** |
||
234 | * Resolve themed javascript path |
||
235 | * |
||
236 | * A javascript file in the current theme path name 'themename/javascript/$name.js' is first searched for, |
||
237 | * and it that doesn't exist and the module parameter is set then a javascript file with that name in |
||
238 | * the module is used. |
||
239 | * |
||
240 | * @param string $name The name of the file - eg '/js/File.js' would have the name 'File' |
||
241 | * @param array $themes List of themes |
||
242 | * @return string Path to resolved javascript file (relative to base dir) |
||
243 | */ |
||
244 | View Code Duplication | public function findThemedJavascript($name, $themes) { |
|
254 | |||
255 | /** |
||
256 | * Resolve a themed resource |
||
257 | * |
||
258 | * A themed resource and be any file that resides in a theme folder. |
||
259 | * |
||
260 | * @param string $resource A file path relative to the root folder of a theme |
||
261 | * @param array $themes An order listed of themes to search |
||
262 | */ |
||
263 | public function findThemedResource($resource, $themes) |
||
281 | |||
282 | /** |
||
283 | * Resolve all themes to the list of root folders relative to site root |
||
284 | * |
||
285 | * @param array $themes List of themes to resolve. Supports named theme sets. |
||
286 | * @return array List of root-relative folders in order of precendence. |
||
287 | */ |
||
288 | public function getThemePaths($themes) { |
||
302 | } |
||
303 |
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.