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 AbstractConditionViewHelper 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 AbstractConditionViewHelper, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
37 | abstract class AbstractConditionViewHelper extends AbstractViewHelper implements NodeFilterInterface |
||
38 | { |
||
39 | |||
40 | /** |
||
41 | * @var boolean |
||
42 | */ |
||
43 | protected $escapeOutput = false; |
||
44 | |||
45 | /** |
||
46 | * Initializes the "then" and "else" arguments |
||
47 | */ |
||
48 | public function initializeArguments() |
||
53 | |||
54 | /** |
||
55 | * Renders <f:then> child if $condition is true, otherwise renders <f:else> child. |
||
56 | * Method which only gets called if the template is not compiled. For static calling, |
||
57 | * the then/else nodes are converted to closures and condition evaluation closures. |
||
58 | * |
||
59 | * @return string the rendered string |
||
60 | * @api |
||
61 | */ |
||
62 | public function render() |
||
69 | |||
70 | /** |
||
71 | * @param array $arguments |
||
72 | * @param \Closure $renderChildrenClosure |
||
73 | * @param RenderingContextInterface $renderingContext |
||
74 | * @return mixed |
||
75 | */ |
||
76 | public static function renderStatic(array $arguments, \Closure $renderChildrenClosure, RenderingContextInterface $renderingContext) |
||
93 | |||
94 | /** |
||
95 | * Static method which can be overridden by subclasses. If a subclass |
||
96 | * requires a different (or faster) decision then this method is the one |
||
97 | * to override and implement. |
||
98 | * |
||
99 | * @param array $arguments |
||
100 | * @param RenderingContextInterface $renderingContext |
||
101 | * @return bool |
||
102 | */ |
||
103 | public static function verdict(array $arguments, RenderingContextInterface $renderingContext) |
||
107 | |||
108 | /** |
||
109 | * Static method which can be overridden by subclasses. If a subclass |
||
110 | * requires a different (or faster) decision then this method is the one |
||
111 | * to override and implement. |
||
112 | * |
||
113 | * Note: method signature does not type-hint that an array is desired, |
||
114 | * and as such, *appears* to accept any input type. There is no type hint |
||
115 | * here for legacy reasons - the signature is kept compatible with third |
||
116 | * party packages which depending on PHP version would error out if this |
||
117 | * signature was not compatible with that of existing and in-production |
||
118 | * subclasses that will be using this base class in the future. Let this |
||
119 | * be a warning if someone considers changing this method signature! |
||
120 | * |
||
121 | * @deprecated Deprecated in favor of ClassName::verdict($arguments, renderingContext), will no longer be called in 3.0 |
||
122 | * @param array|NULL $arguments |
||
123 | * @return boolean |
||
124 | * @api |
||
125 | */ |
||
126 | protected static function evaluateCondition($arguments = null) |
||
130 | |||
131 | /** |
||
132 | * @param array $closures |
||
133 | * @param array $conditionClosures |
||
134 | * @param RenderingContextInterface $renderingContext |
||
135 | * @return string |
||
136 | */ |
||
137 | private static function evaluateElseClosures(array $closures, array $conditionClosures, RenderingContextInterface $renderingContext) |
||
150 | |||
151 | /** |
||
152 | * Returns value of "then" attribute. |
||
153 | * If then attribute is not set, iterates through child nodes and renders ThenViewHelper. |
||
154 | * If then attribute is not set and no ThenViewHelper and no ElseViewHelper is found, all child nodes are rendered |
||
155 | * |
||
156 | * @return mixed rendered ThenViewHelper or contents of <f:if> if no ThenViewHelper was found |
||
157 | * @api |
||
158 | */ |
||
159 | protected function renderThenChild() |
||
184 | |||
185 | /** |
||
186 | * Returns value of "else" attribute. |
||
187 | * If else attribute is not set, iterates through child nodes and renders ElseViewHelper. |
||
188 | * If else attribute is not set and no ElseViewHelper is found, an empty string will be returned. |
||
189 | * |
||
190 | * @return string rendered ElseViewHelper or an empty string if no ThenViewHelper was found |
||
191 | * @api |
||
192 | */ |
||
193 | protected function renderElseChild() |
||
218 | |||
219 | /** |
||
220 | * Condition ViewHelpers disallow child nodes that consist purely of whitespace, |
||
221 | * thus avoiding whitespace in output inside f:if structures but not inside any |
||
222 | * f:then or f:else nodes. |
||
223 | * |
||
224 | * @param NodeInterface $node |
||
225 | * @return bool |
||
226 | */ |
||
227 | View Code Duplication | public function allowsChildNodeType(NodeInterface $node): bool |
|
234 | |||
235 | /** |
||
236 | * The compiled ViewHelper adds two new ViewHelper arguments: __thenClosure and __elseClosure. |
||
237 | * These contain closures which are be executed to render the then(), respectively else() case. |
||
238 | * |
||
239 | * @param string $argumentsName |
||
240 | * @param string $closureName |
||
241 | * @param string $initializationPhpCode |
||
242 | * @param ViewHelperNode $node |
||
243 | * @param TemplateCompiler $compiler |
||
244 | * @return string |
||
245 | */ |
||
246 | public function compile($argumentsName, $closureName, &$initializationPhpCode, ViewHelperNode $node, TemplateCompiler $compiler) |
||
275 | } |
||
276 |
Let’s assume you have a class which uses late-static binding:
}
The code above will run fine in your PHP runtime. However, if you now create a sub-class and call the
getSomeVariable()
on that sub-class, you will receive a runtime error:In the case above, it makes sense to update
SomeClass
to useself
instead: