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 Assets 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 Assets, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
13 | class Assets |
||
14 | { |
||
15 | /** |
||
16 | * @var Repository |
||
17 | */ |
||
18 | protected $config; |
||
19 | |||
20 | /** |
||
21 | * @var HtmlBuilder |
||
22 | */ |
||
23 | protected $htmlBuilder; |
||
24 | |||
25 | /** |
||
26 | * @var array |
||
27 | */ |
||
28 | protected $scripts = []; |
||
29 | |||
30 | /** |
||
31 | * @var array |
||
32 | */ |
||
33 | protected $styles = []; |
||
34 | |||
35 | /** |
||
36 | * @var array |
||
37 | */ |
||
38 | protected $appendedScripts = [ |
||
39 | 'header' => [], |
||
40 | 'footer' => [], |
||
41 | ]; |
||
42 | |||
43 | /** |
||
44 | * @var array |
||
45 | */ |
||
46 | protected $appendedStyles = []; |
||
47 | |||
48 | /** |
||
49 | * @var string |
||
50 | */ |
||
51 | protected $build = ''; |
||
52 | |||
53 | const ASSETS_SCRIPT_POSITION_HEADER = 'header'; |
||
54 | |||
55 | const ASSETS_SCRIPT_POSITION_FOOTER = 'footer'; |
||
56 | |||
57 | /** |
||
58 | * Assets constructor. |
||
59 | * |
||
60 | * @param Repository $config |
||
61 | * @param HtmlBuilder $htmlBuilder |
||
62 | */ |
||
63 | public function __construct(Repository $config, HtmlBuilder $htmlBuilder) |
||
73 | |||
74 | /** |
||
75 | * Add scripts to current module. |
||
76 | * |
||
77 | * @param array $assets |
||
78 | * @return $this |
||
79 | */ |
||
80 | public function addScripts($assets) |
||
86 | |||
87 | /** |
||
88 | * Add Css to current module. |
||
89 | * |
||
90 | * @param string[] $assets |
||
91 | * @return $this |
||
92 | */ |
||
93 | public function addStyles($assets) |
||
99 | |||
100 | /** |
||
101 | * Add styles directly. |
||
102 | * |
||
103 | * @param array|string $assets |
||
104 | * @return $this |
||
105 | */ |
||
106 | public function addStylesDirectly($assets) |
||
119 | |||
120 | /** |
||
121 | * Add scripts directly. |
||
122 | * |
||
123 | * @param string|array $assets |
||
124 | * @param string $location |
||
125 | * @return $this |
||
126 | */ |
||
127 | public function addScriptsDirectly($assets, $location = self::ASSETS_SCRIPT_POSITION_FOOTER) |
||
140 | |||
141 | /** |
||
142 | * Remove Css to current module. |
||
143 | * |
||
144 | * @param array $assets |
||
145 | * @return $this |
||
146 | */ |
||
147 | View Code Duplication | public function removeStyles($assets) |
|
164 | |||
165 | /** |
||
166 | * Add scripts. |
||
167 | * |
||
168 | * @param array $assets |
||
169 | * @return $this |
||
170 | */ |
||
171 | View Code Duplication | public function removeScripts($assets) |
|
188 | |||
189 | /** |
||
190 | * Get All scripts in current module. |
||
191 | * |
||
192 | * @param string $location `header` or `footer` |
||
193 | * @return array |
||
194 | */ |
||
195 | public function getScripts($location = null) |
||
213 | |||
214 | /** |
||
215 | * Get All CSS in current module. |
||
216 | * |
||
217 | * @param array $lastStyles Append last CSS to current module |
||
218 | * @return array |
||
219 | */ |
||
220 | public function getStyles($lastStyles = []) |
||
237 | |||
238 | /** |
||
239 | * Convert script to html. |
||
240 | * |
||
241 | * @param string $name |
||
242 | * @return string|null |
||
243 | */ |
||
244 | public function scriptToHtml($name) |
||
248 | |||
249 | /** |
||
250 | * Convert style to html. |
||
251 | * |
||
252 | * @param string $name |
||
253 | */ |
||
254 | public function styleToHtml($name) |
||
258 | |||
259 | /** |
||
260 | * Get script item. |
||
261 | * |
||
262 | * @param string $location |
||
263 | * @param string $configName |
||
264 | * @param string $script |
||
265 | * @return array |
||
266 | */ |
||
267 | protected function getScriptItem($location, $configName, $script) |
||
277 | |||
278 | /** |
||
279 | * Convert item to html. |
||
280 | * |
||
281 | * @param string $name |
||
282 | * @param string $type |
||
283 | * @return string |
||
284 | */ |
||
285 | protected function itemToHtml($name, $type = 'style') |
||
307 | |||
308 | /** |
||
309 | * @param string $configName |
||
310 | * @return string|array |
||
311 | */ |
||
312 | protected function getSourceUrl($configName) |
||
326 | |||
327 | /** |
||
328 | * @param string $configName |
||
329 | * @return bool |
||
330 | */ |
||
331 | protected function isUsingCdn($configName) |
||
335 | |||
336 | /** |
||
337 | * @param string $configName |
||
338 | * @param string $location |
||
339 | * @return array |
||
340 | */ |
||
341 | protected function getSource($configName, $location = null) |
||
371 | |||
372 | /** |
||
373 | * @return string |
||
374 | */ |
||
375 | public function getBuildVersion() |
||
379 | |||
380 | /** |
||
381 | * @return HtmlBuilder |
||
382 | */ |
||
383 | public function getHtmlBuilder() |
||
387 | |||
388 | /** |
||
389 | * Render assets to header. |
||
390 | * |
||
391 | * @param array $lastStyles |
||
392 | * @return string |
||
393 | * @throws \Throwable |
||
394 | */ |
||
395 | public function renderHeader($lastStyles = []) |
||
403 | |||
404 | /** |
||
405 | * Render assets to footer. |
||
406 | * |
||
407 | * @return string |
||
408 | * @throws \Throwable |
||
409 | */ |
||
410 | public function renderFooter() |
||
416 | } |
||
417 |
Let?s assume that you have the following
foreach
statement:$itemValue
is assigned by reference. This is possible because the expression (in the example$array
) can be used as a reference target.However, if we were to replace
$array
with something different like the result of a function call as inthen assigning by reference is not possible anymore as there is no target that could be modified.
Available Fixes
1. Do not assign by reference
2. Assign to a local variable first
3. Return a reference