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 AbstractViewComponent 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 AbstractViewComponent, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
23 | abstract class AbstractViewComponent |
||
24 | { |
||
25 | |||
26 | /** |
||
27 | * @var ExecHelper |
||
28 | */ |
||
29 | public $exec; |
||
30 | /** |
||
31 | * Message to display when rendering component. Won't be serialised to will only be displayed once. |
||
32 | * @var string |
||
33 | */ |
||
34 | public $flashMessage; |
||
35 | /** |
||
36 | * Error to display when rendering component. Won't be serialised to will only be displayed once. |
||
37 | * @var string |
||
38 | */ |
||
39 | public $flashError; |
||
40 | /** |
||
41 | * If we have a parent in $parent, $handle is the parent's handle/identifier for us |
||
42 | * @var string |
||
43 | */ |
||
44 | public $handle; |
||
45 | /** |
||
46 | * @var ViewState An object containing state elements |
||
47 | */ |
||
48 | protected $state; |
||
49 | /** |
||
50 | * @var AbstractViewComponent |
||
51 | */ |
||
52 | protected $parent; |
||
53 | /** |
||
54 | * @var AbstractViewComponent[] |
||
55 | */ |
||
56 | public $childComponents = [ ]; |
||
57 | |||
58 | /** |
||
59 | * @var AbstractTemplate |
||
60 | */ |
||
61 | protected $template; |
||
62 | |||
63 | /** |
||
64 | * @var LoggerInterface |
||
65 | */ |
||
66 | protected $logger; |
||
67 | |||
68 | /** |
||
69 | * @var array |
||
70 | */ |
||
71 | protected $props; |
||
72 | |||
73 | /** |
||
74 | * If set the render() will skip any processing and immediately return this response |
||
75 | * |
||
76 | * @var Response |
||
77 | */ |
||
78 | private $forceResponse; |
||
79 | |||
80 | /** |
||
81 | * @param null $handle |
||
82 | * @param AbstractViewComponent $parent |
||
83 | * @param ExecHelper $execHelper |
||
84 | * @param LoggerInterface $logger |
||
85 | * @internal param array $initConfig |
||
86 | */ |
||
87 | public function __construct( |
||
111 | |||
112 | /** |
||
113 | * @param string $message |
||
114 | * @param string $level A constant from LogLevel |
||
115 | */ |
||
116 | protected function log( $message, $level ){ |
||
123 | |||
124 | /** |
||
125 | * User this to serialise ViewComponents as extra steps may be added later. |
||
126 | * @return string |
||
127 | */ |
||
128 | public function dehydrate(){ |
||
132 | |||
133 | /** |
||
134 | * Use this to unserialise ViewComponents |
||
135 | * @param $serialised |
||
136 | * @param ExecHelper $execHelper |
||
137 | * @param LoggerInterface $logger |
||
138 | * @return AbstractViewComponent |
||
139 | */ |
||
140 | |||
141 | public static function rehydrate( $serialised, ExecHelper $execHelper, LoggerInterface $logger = null ){ |
||
150 | |||
151 | /** |
||
152 | * @return array |
||
153 | */ |
||
154 | public function __sleep() |
||
163 | |||
164 | /** |
||
165 | * Entry point for rendering a component tree. Call updateView() first. |
||
166 | * @param string|null $execMethodName An optional method on this or a subcomponent to execute before rendering |
||
167 | * @param array|null $execArgs |
||
168 | * @throws \Exception |
||
169 | * @return Response |
||
170 | */ |
||
171 | public function render( $execMethodName = null, array $execArgs = null ) |
||
197 | |||
198 | /** |
||
199 | * Execute a component method within the page or component. |
||
200 | * Called first on a top level component which then passes the call down to the appropriate sub-component (or executes on itself if appropriate). |
||
201 | * @param array|string $methodName A methodname in the format subComponent.anotherSubComponent.methodName. Either dotted string as described, or parts in an array. The top level page component shouldn't be included |
||
202 | * @param array $args |
||
203 | * @throws \Exception |
||
204 | * @return Response |
||
205 | */ |
||
206 | protected function execMethod( $methodName, array $args = null ) |
||
229 | |||
230 | /** |
||
231 | * @param $execMethod |
||
232 | * @return string |
||
233 | */ |
||
234 | public function getExecPath( $execMethod ) |
||
239 | |||
240 | /** |
||
241 | * @return AbstractViewComponent |
||
242 | */ |
||
243 | public function getParent() |
||
247 | |||
248 | |||
249 | /** |
||
250 | * @param $string |
||
251 | */ |
||
252 | protected function setFlashMessage( $string ) |
||
256 | |||
257 | /** |
||
258 | * @param $string |
||
259 | */ |
||
260 | protected function setFlashError( $string ) |
||
264 | |||
265 | /** |
||
266 | * Get the root component of the hierarchy |
||
267 | * |
||
268 | * @return AbstractViewComponent |
||
269 | */ |
||
270 | protected function getRootComponent() |
||
278 | |||
279 | /** |
||
280 | * Load or configure the component's template as necessary. |
||
281 | * Called just before the template is used so can depend on $this->state to select template. |
||
282 | * |
||
283 | * @return void |
||
284 | */ |
||
285 | abstract protected function initTemplate(); |
||
286 | |||
287 | /** |
||
288 | * Initialise $this->state with either a new ViewState or an appropriate subclass |
||
289 | * @return void |
||
290 | */ |
||
291 | abstract protected function initState(); |
||
292 | |||
293 | /** |
||
294 | * Return the this object's path in the current component hierarchy |
||
295 | * @return string |
||
296 | */ |
||
297 | protected function getPath() |
||
308 | |||
309 | /** |
||
310 | * Can create a child component on this component and return it. |
||
311 | * |
||
312 | * @param string $handle |
||
313 | * @param string $type |
||
314 | * @param array $props |
||
315 | * @return AbstractViewComponent |
||
316 | * @throws \Exception |
||
317 | */ |
||
318 | protected function addOrUpdateChild( $handle, $type, array $props = [ ] ) |
||
334 | |||
335 | /** |
||
336 | * Render a child component. |
||
337 | * |
||
338 | * @param $handle |
||
339 | * @return Response |
||
340 | * @throws \Exception |
||
341 | */ |
||
342 | public function renderChild( $handle ) |
||
351 | |||
352 | /** |
||
353 | * Using $this->props and $this->state, optionally update state and create/update child components via addOrUpdateChild(). |
||
354 | * @return void |
||
355 | */ |
||
356 | protected function updateState() |
||
360 | |||
361 | /** |
||
362 | * testInputs() compares a set of named inputs (props or args) in the associative array $inputs with an input specification. |
||
363 | * It MUST be used by implementations' doUpdateState() and *Handler() methods to verify their input. |
||
364 | * |
||
365 | * $inputSpec is an array describing allowed inputs with a similar design to php method sigs. |
||
366 | * The keys are field names, the values are 0 to 2 entry arrays with the following entries: [type,default]. |
||
367 | * Type can be set to null to allow any type, or if there is no default it can be left empty. |
||
368 | * If default is not set then the field is required. If default is null then that us used as the default value. |
||
369 | * As defaults can be any value, it's possible to create an object or callable to use as a default. |
||
370 | * Type can be any of the types described at http://www.php.net/manual/en/function.gettype.php except null or unknown type. In addition it can be any class name, callable, float, bool or int. |
||
371 | * E.g. |
||
372 | * [ |
||
373 | * 'anyTypeRequired'=>[], |
||
374 | * 'anyTypeRequired2'=>[null], |
||
375 | * 'anyTypeOptional'=>[null,null], |
||
376 | * 'boolRequired'=>['bool'], |
||
377 | * 'boolRequired2'=>['boolean'], |
||
378 | * 'intOptional'=>['int',3], |
||
379 | * 'intRequired'=>['integer'], |
||
380 | * 'doubleRequired'=>['double'], |
||
381 | * 'floatRequired'=>['float'], |
||
382 | * 'stringRequired'=>['string'], |
||
383 | * 'arrayRequired'=>['array'], |
||
384 | * 'objectRequired'=>['object'], |
||
385 | * 'resourceRequired'=>['resource'], |
||
386 | * 'callableRequired'=>['callable'], |
||
387 | * 'SomeClassRequired'=>['SomeClass'], |
||
388 | * 'SomeClassOptional'=>['SomeClass',null], |
||
389 | * 'SomeClassWithPrebuiltDefault'=>['SomeClass', new SomeClass( 'something' )], |
||
390 | * ] |
||
391 | * @param array $inputSpec See above |
||
392 | * @param array $inputs |
||
393 | * @throws \Exception |
||
394 | */ |
||
395 | protected function testInputs( array $inputSpec, array &$inputs ) |
||
472 | |||
473 | /** |
||
474 | * Update the full component view tree. |
||
475 | * |
||
476 | * @var array $props |
||
477 | */ |
||
478 | public function updateView( $props ) |
||
483 | |||
484 | /** |
||
485 | * Update the component's properties ('input') array |
||
486 | * |
||
487 | * @var array $props |
||
488 | */ |
||
489 | protected function updateProps( $props ) |
||
494 | |||
495 | protected function forceResponse( Response $response ) |
||
499 | |||
500 | /** |
||
501 | * @param ExecHelper $execHelper |
||
502 | */ |
||
503 | private function setExec( ExecHelper $execHelper ) |
||
511 | |||
512 | /** |
||
513 | * |
||
514 | */ |
||
515 | private function handleDependencyInjection() |
||
528 | |||
529 | /** |
||
530 | * @param LoggerInterface $logger |
||
531 | */ |
||
532 | private function setLogger( LoggerInterface $logger = null ) |
||
542 | } |
||
543 |
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.