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 MetadataCollector 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 MetadataCollector, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
21 | class MetadataCollector |
||
22 | { |
||
23 | /** |
||
24 | * @var DocumentFinder |
||
25 | */ |
||
26 | private $finder; |
||
27 | |||
28 | /** |
||
29 | * @var DocumentParser |
||
30 | */ |
||
31 | private $parser; |
||
32 | |||
33 | /** |
||
34 | * @var CacheProvider |
||
35 | */ |
||
36 | private $cache = null; |
||
37 | |||
38 | /** |
||
39 | * @var bool |
||
40 | */ |
||
41 | private $enableCache = false; |
||
42 | |||
43 | /** |
||
44 | * @var array |
||
45 | */ |
||
46 | private $analysisConfiguration; |
||
47 | |||
48 | /** |
||
49 | * Bundles mappings local cache container. Could be stored as the whole bundle or as single document. |
||
50 | * e.g. AcmeDemoBundle, AcmeDemoBundle:Product. |
||
51 | * |
||
52 | * @var mixed |
||
53 | */ |
||
54 | private $mappings = []; |
||
55 | |||
56 | /** |
||
57 | * @param DocumentFinder $finder For finding documents. |
||
58 | * @param DocumentParser $parser For reading document annotations. |
||
59 | * @param CacheProvider $cache Cache provider to store the meta data for later use. |
||
60 | */ |
||
61 | public function __construct($finder, $parser, $cache = null) |
||
71 | |||
72 | /** |
||
73 | * Enables metadata caching. |
||
74 | * |
||
75 | * @param bool $enableCache |
||
76 | */ |
||
77 | public function setEnableCache($enableCache) |
||
81 | |||
82 | /** |
||
83 | * @param array $config |
||
84 | */ |
||
85 | public function setAnalysisConfiguration(array $config) |
||
89 | |||
90 | /** |
||
91 | * Fetches bundles mapping from documents. |
||
92 | * |
||
93 | * @param string[] $bundles Elasticsearch manager config. You can get bundles list from 'mappings' node. |
||
94 | * @return array |
||
95 | */ |
||
96 | public function getMappings(array $bundles) |
||
116 | |||
117 | /** |
||
118 | * Searches for documents in the bundle and tries to read them. |
||
119 | * |
||
120 | * @param string $name |
||
121 | * |
||
122 | * @return array Empty array on containing zero documents. |
||
123 | */ |
||
124 | public function getBundleMapping($name) |
||
182 | |||
183 | /** |
||
184 | * @param array $manager |
||
185 | * |
||
186 | * @return array |
||
187 | */ |
||
188 | public function getManagerTypes($manager) |
||
194 | |||
195 | /** |
||
196 | * Resolves Elasticsearch type by document class. |
||
197 | * |
||
198 | * @param string $className FQCN or string in AppBundle:Document format |
||
199 | * |
||
200 | * @return string |
||
201 | */ |
||
202 | public function getDocumentType($className) |
||
208 | |||
209 | /** |
||
210 | * Retrieves prepared mapping to sent to the elasticsearch client. |
||
211 | * |
||
212 | * @param array $bundles Manager config. |
||
213 | * |
||
214 | * @return array|null |
||
215 | */ |
||
216 | public function getClientMapping(array $bundles) |
||
240 | |||
241 | /** |
||
242 | * Gets the analysis configuration from the documents of the specific bundles |
||
243 | * |
||
244 | * @param string $manager Manager name |
||
245 | * @param array $bundles |
||
246 | * |
||
247 | * @return array |
||
248 | */ |
||
249 | public function getManagerAnalysis($manager, array $bundles) |
||
273 | |||
274 | /** |
||
275 | * Gathers annotation data from class. |
||
276 | * |
||
277 | * @param \ReflectionClass $reflectionClass Document reflection class to read mapping from. |
||
278 | * |
||
279 | * @return array |
||
280 | * @throws DocumentParserException |
||
281 | */ |
||
282 | private function getDocumentReflectionMapping(\ReflectionClass $reflectionClass) |
||
286 | |||
287 | /** |
||
288 | * Returns single document mapping metadata. |
||
289 | * |
||
290 | * @param string $namespace Document namespace |
||
291 | * |
||
292 | * @return array |
||
293 | * @throws DocumentParserException |
||
294 | */ |
||
295 | public function getMapping($namespace) |
||
308 | |||
309 | /** |
||
310 | * Adds metadata information to the cache storage. |
||
311 | * |
||
312 | * @param string $bundle |
||
313 | * @param array $mapping |
||
314 | */ |
||
315 | private function cacheBundle($bundle, array $mapping) |
||
322 | |||
323 | /** |
||
324 | * Returns fully qualified class name. |
||
325 | * |
||
326 | * @param string $className |
||
327 | * |
||
328 | * @return string |
||
329 | */ |
||
330 | public function getClassName($className) |
||
334 | |||
335 | /** |
||
336 | * Extracts analysis configuration from all the documents |
||
337 | * |
||
338 | * @param array $properties Properties of a type or an object |
||
339 | * @param array $managerAnalysis The data that is being formed for the manager |
||
340 | */ |
||
341 | private function extractAnalysisFromProperties($properties, &$managerAnalysis) |
||
358 | |||
359 | /** |
||
360 | * Extracts tokenizers and filters from analysis configuration |
||
361 | * |
||
362 | * @param string $type Either filter or tokenizer |
||
363 | * @param array $analyzer The current analyzer |
||
364 | * @param array $data The data that is being formed for the manager |
||
365 | */ |
||
366 | private function extractSubData($type, $analyzer, &$data) |
||
386 | } |
||
387 |
There are different options of fixing this problem.
If you want to be on the safe side, you can add an additional type-check:
If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:
Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.