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 VariableExtractor 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 VariableExtractor, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
16 | class VariableExtractor |
||
17 | { |
||
18 | |||
19 | const ACCESSOR_ARRAY = 'array'; |
||
20 | const ACCESSOR_GETTER = 'getter'; |
||
21 | const ACCESSOR_ASSERTER = 'asserter'; |
||
22 | const ACCESSOR_PUBLICPROPERTY = 'public'; |
||
23 | |||
24 | /** |
||
25 | * Static interface for instanciating and extracting |
||
26 | * in a single operation. Delegates to getByPath. |
||
27 | * |
||
28 | * @param mixed $subject |
||
29 | * @param string $propertyPath |
||
30 | * @param array $accessors |
||
31 | * @return mixed |
||
32 | */ |
||
33 | public static function extract($subject, $propertyPath, array $accessors = []) |
||
38 | |||
39 | /** |
||
40 | * Static interface for instanciating and extracting |
||
41 | * accessors for each segment of the path. |
||
42 | * |
||
43 | * @param VariableProviderInterface $subject |
||
44 | * @param string $propertyPath |
||
45 | * @return mixed |
||
46 | */ |
||
47 | public static function extractAccessors($subject, $propertyPath) |
||
52 | |||
53 | /** |
||
54 | * Extracts a variable by path, recursively, from the |
||
55 | * subject pass in argument. This implementation supports |
||
56 | * recursive variable references by using {} around sub- |
||
57 | * references, e.g. "array.{index}" will first get the |
||
58 | * "array" variable, then resolve the "index" variable |
||
59 | * before using the value of "index" as name of the property |
||
60 | * to return. So: |
||
61 | * |
||
62 | * $subject = array('foo' => array('bar' => 'baz'), 'key' => 'bar') |
||
63 | * $propertyPath = 'foo.{key}'; |
||
64 | * $result = ...getByPath($subject, $propertyPath); |
||
65 | * // $result value is "baz", because $subject['foo'][$subject['key']] = 'baz'; |
||
66 | * |
||
67 | * @param mixed $subject |
||
68 | * @param string $propertyPath |
||
69 | * @param array $accessors |
||
70 | * @return mixed |
||
71 | */ |
||
72 | public function getByPath($subject, $propertyPath, array $accessors = []) |
||
89 | |||
90 | /** |
||
91 | * @param VariableProviderInterface $subject |
||
92 | * @param string $propertyPath |
||
93 | * @return array |
||
94 | */ |
||
95 | public function getAccessorsForPath($subject, $propertyPath) |
||
113 | |||
114 | /** |
||
115 | * @param mixed $subject |
||
116 | * @param string $propertyPath |
||
117 | * @return string |
||
118 | */ |
||
119 | protected function resolveSubVariableReferences($subject, $propertyPath) |
||
130 | |||
131 | /** |
||
132 | * Extracts a single value from an array or object. |
||
133 | * |
||
134 | * @param mixed $subject |
||
135 | * @param string $propertyName |
||
136 | * @param string|null $accessor |
||
137 | * @return mixed |
||
138 | */ |
||
139 | protected function extractSingleValue($subject, $propertyName, $accessor = null) |
||
146 | |||
147 | /** |
||
148 | * Returns TRUE if the data type of $subject is potentially compatible |
||
149 | * with the $accessor. |
||
150 | * |
||
151 | * @param mixed $subject |
||
152 | * @param string $propertyName |
||
153 | * @param string $accessor |
||
154 | * @return boolean |
||
155 | */ |
||
156 | protected function canExtractWithAccessor($subject, $propertyName, $accessor) |
||
170 | |||
171 | /** |
||
172 | * @param mixed $subject |
||
173 | * @param string $propertyName |
||
174 | * @param string $accessor |
||
175 | * @return mixed |
||
176 | */ |
||
177 | protected function extractWithAccessor($subject, $propertyName, $accessor) |
||
194 | |||
195 | /** |
||
196 | * Detect which type of accessor to use when extracting |
||
197 | * $propertyName from $subject. |
||
198 | * |
||
199 | * @param mixed $subject |
||
200 | * @param string $propertyName |
||
201 | * @return string|NULL |
||
202 | */ |
||
203 | protected function detectAccessor($subject, $propertyName) |
||
224 | |||
225 | /** |
||
226 | * Tests whether a property can be extracted through `is*` or `has*` methods. |
||
227 | * |
||
228 | * @param mixed $subject |
||
229 | * @param string $propertyName |
||
230 | * @return bool |
||
231 | */ |
||
232 | protected function isExtractableThroughAsserter($subject, $propertyName) |
||
237 | |||
238 | /** |
||
239 | * Extracts a property through `is*` or `has*` methods. |
||
240 | * |
||
241 | * @param object $subject |
||
242 | * @param string $propertyName |
||
243 | * @return mixed |
||
244 | */ |
||
245 | protected function extractThroughAsserter($subject, $propertyName) |
||
253 | } |
||
254 |
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.