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 RuleSetFactory 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 RuleSetFactory, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
24 | class RuleSetFactory |
||
25 | { |
||
26 | /** |
||
27 | * Is the strict mode active? |
||
28 | * |
||
29 | * @var boolean |
||
30 | * @since 1.2.0 |
||
31 | */ |
||
32 | private $strict = false; |
||
33 | |||
34 | /** |
||
35 | * The data directory set by PEAR or a dynamic property set within the class |
||
36 | * constructor. |
||
37 | * |
||
38 | * @var string |
||
39 | */ |
||
40 | private $location = '@data_dir@'; |
||
41 | |||
42 | /** |
||
43 | * The minimum priority for rules to load. |
||
44 | * |
||
45 | * @var integer |
||
46 | */ |
||
47 | private $minimumPriority = Rule::LOWEST_PRIORITY; |
||
48 | |||
49 | /** |
||
50 | * The maximum priority for rules to load. |
||
51 | * |
||
52 | * @var integer |
||
53 | */ |
||
54 | private $maximumPriority = Rule::HIGHEST_PRIORITY; |
||
55 | |||
56 | /** |
||
57 | * Constructs a new default rule-set factory instance. |
||
58 | */ |
||
59 | 48 | public function __construct() |
|
60 | { |
||
61 | // PEAR installer workaround |
||
62 | 48 | if (strpos($this->location, '@data_dir') === 0) { |
|
63 | 48 | $this->location = __DIR__ . '/../../resources'; |
|
64 | } else { |
||
65 | $this->location .= '/PHPMD/resources'; |
||
66 | } |
||
67 | 48 | } |
|
68 | |||
69 | /** |
||
70 | * Activates the strict mode for all rule sets. |
||
71 | * |
||
72 | * @return void |
||
73 | * @since 1.2.0 |
||
74 | */ |
||
75 | 1 | public function setStrict() |
|
76 | { |
||
77 | 1 | $this->strict = true; |
|
78 | 1 | } |
|
79 | |||
80 | /** |
||
81 | * Sets the minimum priority that a rule must have. |
||
82 | * |
||
83 | * @param integer $minimumPriority The minimum priority value. |
||
84 | * @return void |
||
85 | */ |
||
86 | 12 | public function setMinimumPriority($minimumPriority) |
|
87 | { |
||
88 | 12 | $this->minimumPriority = $minimumPriority; |
|
89 | 12 | } |
|
90 | |||
91 | /** |
||
92 | * Sets the maximum priority that a rule must have. |
||
93 | * |
||
94 | * @param integer $maximumPriority The maximum priority value. |
||
95 | * @return void |
||
96 | */ |
||
97 | 12 | public function setMaximumPriority($maximumPriority) |
|
98 | { |
||
99 | 12 | $this->maximumPriority = $maximumPriority; |
|
100 | 12 | } |
|
101 | |||
102 | /** |
||
103 | * Creates an array of rule-set instances for the given argument. |
||
104 | * |
||
105 | * @param string $ruleSetFileNames Comma-separated string of rule-set filenames or identifier. |
||
106 | * @return \PHPMD\RuleSet[] |
||
107 | */ |
||
108 | 40 | public function createRuleSets($ruleSetFileNames) |
|
109 | { |
||
110 | 40 | $ruleSets = array(); |
|
111 | |||
112 | 40 | $ruleSetFileName = strtok($ruleSetFileNames, ','); |
|
113 | 40 | while ($ruleSetFileName !== false) { |
|
114 | 40 | $ruleSets[] = $this->createSingleRuleSet($ruleSetFileName); |
|
115 | |||
116 | 36 | $ruleSetFileName = strtok(','); |
|
117 | } |
||
118 | 36 | return $ruleSets; |
|
119 | } |
||
120 | |||
121 | /** |
||
122 | * Creates a single rule-set instance for the given filename or identifier. |
||
123 | * |
||
124 | * @param string $ruleSetOrFileName The rule-set filename or identifier. |
||
125 | * @return \PHPMD\RuleSet |
||
126 | */ |
||
127 | 46 | public function createSingleRuleSet($ruleSetOrFileName) |
|
128 | { |
||
129 | 46 | $fileName = $this->createRuleSetFileName($ruleSetOrFileName); |
|
130 | 45 | return $this->parseRuleSetNode($fileName); |
|
131 | } |
||
132 | |||
133 | /** |
||
134 | * Lists available rule-set identifiers. |
||
135 | * |
||
136 | * @return string[] |
||
137 | */ |
||
138 | 4 | public function listAvailableRuleSets() |
|
145 | |||
146 | /** |
||
147 | * This method creates the filename for a rule-set identifier or it returns |
||
148 | * the input when it is already a filename. |
||
149 | * |
||
150 | * @param string $ruleSetOrFileName The rule-set filename or identifier. |
||
151 | * @return string Path to rule set file name |
||
152 | * @throws RuleSetNotFoundException Thrown if no readable file found |
||
153 | */ |
||
154 | 48 | private function createRuleSetFileName($ruleSetOrFileName) |
|
155 | { |
||
156 | 48 | foreach ($this->filePaths($ruleSetOrFileName) as $filePath) { |
|
157 | 48 | if ($this->isReadableFile($filePath)) { |
|
158 | 48 | return $filePath; |
|
159 | } |
||
160 | } |
||
161 | |||
162 | 2 | throw new RuleSetNotFoundException($ruleSetOrFileName); |
|
163 | } |
||
164 | |||
165 | /** |
||
166 | * Lists available rule-set identifiers in given directory. |
||
167 | * |
||
168 | * @param string $directory The directory to scan for rule-sets. |
||
169 | * @return string[] |
||
170 | */ |
||
171 | 4 | private static function listRuleSetsInDirectory($directory) |
|
172 | { |
||
173 | 4 | $ruleSets = array(); |
|
174 | 4 | if (is_dir($directory)) { |
|
175 | 4 | foreach (scandir($directory) as $file) { |
|
176 | 4 | $matches = array(); |
|
177 | 4 | if (is_file($directory . $file) && preg_match('/^(.*)\.xml$/', $file, $matches)) { |
|
178 | 4 | $ruleSets[] = $matches[1]; |
|
179 | } |
||
180 | } |
||
181 | } |
||
182 | 4 | return $ruleSets; |
|
183 | } |
||
184 | |||
185 | /** |
||
186 | * This method parses the rule-set definition in the given file. |
||
187 | * |
||
188 | * @param string $fileName |
||
189 | * @return \PHPMD\RuleSet |
||
190 | */ |
||
191 | 45 | private function parseRuleSetNode($fileName) |
|
192 | { |
||
193 | // Hide error messages |
||
194 | 45 | $libxml = libxml_use_internal_errors(true); |
|
195 | |||
196 | 45 | $xml = simplexml_load_string(file_get_contents($fileName)); |
|
197 | 45 | View Code Duplication | if ($xml === false) { |
|
|||
198 | // Reset error handling to previous setting |
||
199 | 1 | libxml_use_internal_errors($libxml); |
|
200 | |||
201 | 1 | throw new \RuntimeException(trim(libxml_get_last_error()->message)); |
|
202 | } |
||
203 | |||
204 | 44 | $ruleSet = new RuleSet(); |
|
205 | 44 | $ruleSet->setFileName($fileName); |
|
206 | 44 | $ruleSet->setName((string) $xml['name']); |
|
207 | |||
208 | 44 | if ($this->strict) { |
|
209 | 1 | $ruleSet->setStrict(); |
|
210 | } |
||
211 | |||
212 | 44 | foreach ($xml->children() as $node) { |
|
213 | /** @var $node \SimpleXMLElement */ |
||
214 | 44 | if ($node->getName() === 'php-includepath') { |
|
215 | 1 | $includePath = (string) $node; |
|
216 | |||
217 | 1 | if (is_dir(dirname($fileName) . DIRECTORY_SEPARATOR . $includePath)) { |
|
218 | $includePath = dirname($fileName) . DIRECTORY_SEPARATOR . $includePath; |
||
219 | $includePath = realpath($includePath); |
||
220 | } |
||
221 | |||
222 | 1 | $includePath = get_include_path() . PATH_SEPARATOR . $includePath; |
|
223 | 44 | set_include_path($includePath); |
|
224 | } |
||
225 | } |
||
226 | |||
227 | 44 | foreach ($xml->children() as $node) { |
|
228 | 44 | if ($node->getName() === 'description') { |
|
229 | 44 | $ruleSet->setDescription((string) $node); |
|
230 | 44 | } elseif ($node->getName() === 'rule') { |
|
231 | 44 | $this->parseRuleNode($ruleSet, $node); |
|
232 | } |
||
233 | } |
||
234 | |||
235 | 42 | return $ruleSet; |
|
236 | } |
||
237 | |||
238 | /** |
||
239 | * This method parses a single rule xml node. Bases on the structure of the |
||
240 | * xml node this method delegates the parsing process to another method in |
||
241 | * this class. |
||
242 | * |
||
243 | * @param \PHPMD\RuleSet $ruleSet |
||
244 | * @param \SimpleXMLElement $node |
||
245 | * @return void |
||
246 | */ |
||
247 | 44 | private function parseRuleNode(RuleSet $ruleSet, \SimpleXMLElement $node) |
|
248 | { |
||
249 | 44 | if (substr($node['ref'], -3, 3) === 'xml') { |
|
250 | 6 | $this->parseRuleSetReferenceNode($ruleSet, $node); |
|
251 | 44 | } elseif ('' === (string) $node['ref']) { |
|
252 | 44 | $this->parseSingleRuleNode($ruleSet, $node); |
|
253 | } else { |
||
254 | 8 | $this->parseRuleReferenceNode($ruleSet, $node); |
|
255 | } |
||
256 | 42 | } |
|
257 | |||
258 | /** |
||
259 | * This method parses a complete rule set that was includes a reference in |
||
260 | * the currently parsed ruleset. |
||
261 | * |
||
262 | * @param \PHPMD\RuleSet $ruleSet |
||
263 | * @param \SimpleXMLElement $ruleSetNode |
||
264 | * @return void |
||
265 | */ |
||
266 | 6 | private function parseRuleSetReferenceNode(RuleSet $ruleSet, \SimpleXMLElement $ruleSetNode) |
|
267 | { |
||
268 | 6 | $rules = $this->parseRuleSetReference($ruleSetNode); |
|
269 | 6 | foreach ($rules as $rule) { |
|
270 | 6 | if ($this->isIncluded($rule, $ruleSetNode)) { |
|
271 | 6 | $ruleSet->addRule($rule); |
|
272 | } |
||
273 | } |
||
274 | 6 | } |
|
275 | |||
276 | /** |
||
277 | * Parses a rule-set xml file referenced by the given rule-set xml element. |
||
278 | * |
||
279 | * @param \SimpleXMLElement $ruleSetNode |
||
280 | * @return \PHPMD\RuleSet |
||
281 | * @since 0.2.3 |
||
282 | */ |
||
283 | 6 | private function parseRuleSetReference(\SimpleXMLElement $ruleSetNode) |
|
284 | { |
||
285 | 6 | $ruleSetFactory = new RuleSetFactory(); |
|
286 | 6 | $ruleSetFactory->setMinimumPriority($this->minimumPriority); |
|
287 | 6 | $ruleSetFactory->setMaximumPriority($this->maximumPriority); |
|
288 | |||
289 | 6 | return $ruleSetFactory->createSingleRuleSet((string) $ruleSetNode['ref']); |
|
290 | } |
||
291 | |||
292 | /** |
||
293 | * Checks if the given rule is included/not excluded by the given rule-set |
||
294 | * reference node. |
||
295 | * |
||
296 | * @param \PHPMD\Rule $rule |
||
297 | * @param \SimpleXMLElement $ruleSetNode |
||
298 | * @return boolean |
||
299 | * @since 0.2.3 |
||
300 | */ |
||
301 | 6 | private function isIncluded(Rule $rule, \SimpleXMLElement $ruleSetNode) |
|
310 | |||
311 | /** |
||
312 | * This method will create a single rule instance and add it to the given |
||
313 | * {@link \PHPMD\RuleSet} object. |
||
314 | * |
||
315 | * @param \PHPMD\RuleSet $ruleSet |
||
316 | * @param \SimpleXMLElement $ruleNode |
||
317 | * @return void |
||
318 | * @throws \PHPMD\RuleClassFileNotFoundException |
||
319 | * @throws \PHPMD\RuleClassNotFoundException |
||
320 | */ |
||
321 | 44 | private function parseSingleRuleNode(RuleSet $ruleSet, \SimpleXMLElement $ruleNode) |
|
322 | { |
||
323 | 44 | $fileName = ""; |
|
324 | |||
325 | 44 | $ruleSetFolderPath = dirname($ruleSet->getFileName()); |
|
326 | |||
327 | 44 | if (isset($ruleNode['file'])) { |
|
328 | 1 | if (is_readable((string) $ruleNode['file'])) { |
|
329 | $fileName = (string) $ruleNode['file']; |
||
330 | 1 | } elseif (is_readable($ruleSetFolderPath . DIRECTORY_SEPARATOR . (string) $ruleNode['file'])) { |
|
331 | 1 | $fileName = $ruleSetFolderPath . DIRECTORY_SEPARATOR . (string) $ruleNode['file']; |
|
332 | } |
||
333 | } |
||
334 | |||
335 | 44 | $className = (string) $ruleNode['class']; |
|
336 | |||
337 | 44 | if (!is_readable($fileName)) { |
|
338 | 44 | $fileName = strtr($className, '\\', '/') . '.php'; |
|
339 | } |
||
340 | |||
341 | 44 | if (!is_readable($fileName)) { |
|
342 | 44 | $fileName = str_replace(array('\\', '_'), '/', $className) . '.php'; |
|
343 | } |
||
344 | |||
345 | 44 | if (class_exists($className) === false) { |
|
346 | 3 | $handle = @fopen($fileName, 'r', true); |
|
347 | 3 | if ($handle === false) { |
|
348 | 1 | throw new RuleClassFileNotFoundException($className); |
|
349 | } |
||
350 | 2 | fclose($handle); |
|
351 | |||
352 | 2 | include_once $fileName; |
|
353 | |||
354 | 2 | if (class_exists($className) === false) { |
|
355 | 1 | throw new RuleClassNotFoundException($className); |
|
356 | } |
||
357 | } |
||
358 | |||
359 | /* @var $rule \PHPMD\Rule */ |
||
360 | 42 | $rule = new $className(); |
|
361 | 42 | $rule->setName((string) $ruleNode['name']); |
|
362 | 42 | $rule->setMessage((string) $ruleNode['message']); |
|
363 | 42 | $rule->setExternalInfoUrl((string) $ruleNode['externalInfoUrl']); |
|
364 | |||
365 | 42 | $rule->setRuleSetName($ruleSet->getName()); |
|
366 | |||
367 | 42 | if (trim($ruleNode['since']) !== '') { |
|
368 | 42 | $rule->setSince((string) $ruleNode['since']); |
|
369 | } |
||
370 | |||
371 | 42 | View Code Duplication | foreach ($ruleNode->children() as $node) { |
372 | /** @var $node \SimpleXMLElement */ |
||
373 | 42 | if ($node->getName() === 'description') { |
|
374 | 42 | $rule->setDescription((string) $node); |
|
375 | 42 | } elseif ($node->getName() === 'example') { |
|
376 | 41 | $rule->addExample((string) $node); |
|
377 | 42 | } elseif ($node->getName() === 'priority') { |
|
378 | 42 | $rule->setPriority((integer) $node); |
|
379 | 42 | } elseif ($node->getName() === 'properties') { |
|
380 | 42 | $this->parsePropertiesNode($rule, $node); |
|
381 | } |
||
382 | } |
||
383 | |||
384 | 42 | if ($rule->getPriority() <= $this->minimumPriority && $rule->getPriority() >= $this->maximumPriority) { |
|
385 | 41 | $ruleSet->addRule($rule); |
|
386 | } |
||
387 | 42 | } |
|
388 | |||
389 | /** |
||
390 | * This method parses a single rule that was included from a different |
||
391 | * rule-set. |
||
392 | * |
||
393 | * @param \PHPMD\RuleSet $ruleSet |
||
394 | * @param \SimpleXMLElement $ruleNode |
||
395 | * @return void |
||
396 | */ |
||
397 | 8 | private function parseRuleReferenceNode(RuleSet $ruleSet, \SimpleXMLElement $ruleNode) |
|
398 | { |
||
399 | 8 | $ref = (string) $ruleNode['ref']; |
|
400 | |||
401 | 8 | $fileName = substr($ref, 0, strpos($ref, '.xml/') + 4); |
|
402 | 8 | $fileName = $this->createRuleSetFileName($fileName); |
|
403 | |||
404 | 8 | $ruleName = substr($ref, strpos($ref, '.xml/') + 5); |
|
405 | |||
406 | 8 | $ruleSetFactory = new RuleSetFactory(); |
|
407 | |||
408 | 8 | $ruleSetRef = $ruleSetFactory->createSingleRuleSet($fileName); |
|
409 | 8 | $rule = $ruleSetRef->getRuleByName($ruleName); |
|
410 | |||
411 | 8 | if (trim($ruleNode['name']) !== '') { |
|
412 | 3 | $rule->setName((string) $ruleNode['name']); |
|
413 | } |
||
414 | 8 | if (trim($ruleNode['message']) !== '') { |
|
415 | 3 | $rule->setMessage((string) $ruleNode['message']); |
|
416 | } |
||
417 | 8 | if (trim($ruleNode['externalInfoUrl']) !== '') { |
|
418 | 3 | $rule->setExternalInfoUrl((string) $ruleNode['externalInfoUrl']); |
|
419 | } |
||
420 | |||
421 | 8 | View Code Duplication | foreach ($ruleNode->children() as $node) { |
422 | /** @var $node \SimpleXMLElement */ |
||
423 | 4 | if ($node->getName() === 'description') { |
|
424 | 4 | $rule->setDescription((string) $node); |
|
425 | 4 | } elseif ($node->getName() === 'example') { |
|
426 | 4 | $rule->addExample((string) $node); |
|
427 | 4 | } elseif ($node->getName() === 'priority') { |
|
428 | 4 | $rule->setPriority((integer) $node); |
|
429 | 4 | } elseif ($node->getName() === 'properties') { |
|
430 | 4 | $this->parsePropertiesNode($rule, $node); |
|
431 | } |
||
432 | } |
||
433 | |||
434 | 8 | if ($rule->getPriority() <= $this->minimumPriority && $rule->getPriority() >= $this->maximumPriority) { |
|
435 | 8 | $ruleSet->addRule($rule); |
|
436 | } |
||
437 | 8 | } |
|
438 | |||
439 | /** |
||
440 | * This method parses a xml properties structure and adds all found properties |
||
441 | * to the given <b>$rule</b> object. |
||
442 | * |
||
443 | * <code> |
||
444 | * ... |
||
445 | * <properties> |
||
446 | * <property name="foo" value="42" /> |
||
447 | * <property name="bar" value="23" /> |
||
448 | * ... |
||
449 | * </properties> |
||
450 | * ... |
||
451 | * </code> |
||
452 | * |
||
453 | * @param \PHPMD\Rule $rule |
||
454 | * @param \SimpleXMLElement $propertiesNode |
||
455 | * @return void |
||
456 | */ |
||
457 | 42 | private function parsePropertiesNode(Rule $rule, \SimpleXMLElement $propertiesNode) |
|
466 | |||
467 | /** |
||
468 | * Adds an additional property to the given <b>$rule</b> instance. |
||
469 | * |
||
470 | * @param \PHPMD\Rule $rule |
||
471 | * @param \SimpleXMLElement $node |
||
472 | * @return void |
||
473 | */ |
||
474 | 12 | private function addProperty(Rule $rule, \SimpleXMLElement $node) |
|
475 | { |
||
476 | 12 | $name = trim($node['name']); |
|
477 | 12 | $value = trim($this->getPropertyValue($node)); |
|
478 | 12 | if ($name !== '' && $value !== '') { |
|
479 | 12 | $rule->addProperty($name, $value); |
|
480 | } |
||
481 | 12 | } |
|
482 | |||
483 | /** |
||
484 | * Returns the value of a property node. This value can be expressed in |
||
485 | * two different notations. First version is an attribute named <b>value</b> |
||
486 | * and the second valid notation is a child element named <b>value</b> that |
||
487 | * contains the value as character data. |
||
488 | * |
||
489 | * @param \SimpleXMLElement $propertyNode |
||
490 | * @return string |
||
491 | * @since 0.2.5 |
||
492 | */ |
||
493 | 12 | private function getPropertyValue(\SimpleXMLElement $propertyNode) |
|
500 | |||
501 | /** |
||
502 | * Returns an array of path exclude patterns in format described at |
||
503 | * |
||
504 | * http://pmd.sourceforge.net/pmd-5.0.4/howtomakearuleset.html#Excluding_files_from_a_ruleset |
||
505 | * |
||
506 | * @param string $fileName The filename of a rule-set definition. |
||
507 | * @return array|null |
||
508 | * @throws \RuntimeException Thrown if file is not proper xml |
||
509 | * @throws RuleSetNotFoundException Thrown if no readable file found |
||
510 | */ |
||
511 | 8 | public function getIgnorePattern($fileName) |
|
512 | { |
||
513 | 8 | $excludes = array(); |
|
514 | 8 | foreach (array_map('trim', explode(',', $fileName)) as $ruleSetFileName) { |
|
515 | 8 | $ruleSetFileName = $this->createRuleSetFileName($ruleSetFileName); |
|
539 | |||
540 | /** |
||
541 | * Checks if given file path exists, is file (or symlink to file) |
||
542 | * and is readable by current user |
||
543 | * |
||
544 | * @param string $filePath File path to check against |
||
545 | * @return bool True if file exists and is readable, false otherwise |
||
546 | */ |
||
547 | 48 | private function isReadableFile($filePath) |
|
554 | |||
555 | /** |
||
556 | * Returns list of possible file paths to search against code rules |
||
557 | * |
||
558 | * @param string $fileName Rule set file name |
||
559 | * @return array Array of possible file locations |
||
560 | */ |
||
561 | 48 | private function filePaths($fileName) |
|
577 | } |
||
578 |
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.