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 XmlDomParser 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 XmlDomParser, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
16 | class XmlDomParser extends AbstractDomParser |
||
17 | { |
||
18 | /** |
||
19 | * @param \DOMNode|SimpleXmlDomInterface|string $element HTML code or SimpleXmlDomInterface, \DOMNode |
||
20 | */ |
||
21 | 3 | public function __construct($element = null) |
|
22 | { |
||
23 | 3 | $this->document = new \DOMDocument('1.0', $this->getEncoding()); |
|
24 | |||
25 | // DOMDocument settings |
||
26 | 3 | $this->document->preserveWhiteSpace = true; |
|
27 | 3 | $this->document->formatOutput = true; |
|
28 | |||
29 | 3 | if ($element instanceof SimpleXmlDomInterface) { |
|
30 | $element = $element->getNode(); |
||
31 | } |
||
32 | |||
33 | 3 | if ($element instanceof \DOMNode) { |
|
34 | $domNode = $this->document->importNode($element, true); |
||
35 | |||
36 | if ($domNode instanceof \DOMNode) { |
||
37 | /** @noinspection UnusedFunctionResultInspection */ |
||
38 | $this->document->appendChild($domNode); |
||
39 | } |
||
40 | |||
41 | return; |
||
42 | } |
||
43 | |||
44 | 3 | if ($element !== null) { |
|
45 | $this->loadXml($element); |
||
46 | } |
||
47 | 3 | } |
|
48 | |||
49 | /** |
||
50 | * @param string $name |
||
51 | * @param array $arguments |
||
52 | * |
||
53 | * @throws \BadMethodCallException |
||
54 | * @throws \RuntimeException |
||
55 | * |
||
56 | * @return XmlDomParser |
||
57 | */ |
||
58 | 3 | public static function __callStatic($name, $arguments) |
|
59 | { |
||
60 | 3 | $arguments0 = $arguments[0] ?? ''; |
|
61 | |||
62 | 3 | $arguments1 = $arguments[1] ?? null; |
|
63 | |||
64 | 3 | if ($name === 'str_get_xml') { |
|
65 | 1 | $parser = new static(); |
|
66 | |||
67 | 1 | return $parser->loadXml($arguments0, $arguments1); |
|
68 | } |
||
69 | |||
70 | 2 | if ($name === 'file_get_xml') { |
|
71 | 2 | $parser = new static(); |
|
72 | |||
73 | 2 | return $parser->loadXmlFile($arguments0, $arguments1); |
|
|
|||
74 | } |
||
75 | |||
76 | throw new \BadMethodCallException('Method does not exist'); |
||
77 | } |
||
78 | |||
79 | /** @noinspection MagicMethodsValidityInspection */ |
||
80 | |||
81 | /** |
||
82 | * @param string $name |
||
83 | * |
||
84 | * @return string|null |
||
85 | */ |
||
86 | public function __get($name) |
||
96 | |||
97 | /** |
||
98 | * @return string |
||
99 | */ |
||
100 | 2 | public function __toString() |
|
104 | |||
105 | /** |
||
106 | * Create DOMDocument from XML. |
||
107 | * |
||
108 | * @param string $xml |
||
109 | * @param int|null $libXMLExtraOptions |
||
110 | * |
||
111 | * @return \DOMDocument |
||
112 | */ |
||
113 | 3 | protected function createDOMDocument(string $xml, $libXMLExtraOptions = null): \DOMDocument |
|
189 | |||
190 | /** |
||
191 | * Find list of nodes with a CSS selector. |
||
192 | * |
||
193 | * @param string $selector |
||
194 | * @param int|null $idx |
||
195 | * |
||
196 | * @return SimpleXmlDomInterface|SimpleXmlDomInterface[]|SimpleXmlDomNodeInterface<SimpleXmlDomInterface> |
||
197 | */ |
||
198 | 1 | public function find(string $selector, $idx = null) |
|
229 | |||
230 | /** |
||
231 | * Find nodes with a CSS selector. |
||
232 | * |
||
233 | * @param string $selector |
||
234 | * |
||
235 | * @return SimpleXmlDomInterface[]|SimpleXmlDomNodeInterface<SimpleXmlDomInterface> |
||
236 | */ |
||
237 | 1 | public function findMulti(string $selector): SimpleXmlDomNodeInterface |
|
241 | |||
242 | /** |
||
243 | * Find nodes with a CSS selector or false, if no element is found. |
||
244 | * |
||
245 | * @param string $selector |
||
246 | * |
||
247 | * @return false|SimpleXmlDomInterface[]|SimpleXmlDomNodeInterface<SimpleXmlDomInterface> |
||
248 | */ |
||
249 | 1 | public function findMultiOrFalse(string $selector) |
|
259 | |||
260 | /** |
||
261 | * Find one node with a CSS selector. |
||
262 | * |
||
263 | * @param string $selector |
||
264 | * |
||
265 | * @return SimpleXmlDomInterface |
||
266 | */ |
||
267 | 1 | public function findOne(string $selector): SimpleXmlDomInterface |
|
271 | |||
272 | /** |
||
273 | * Find one node with a CSS selector or false, if no element is found. |
||
274 | * |
||
275 | * @param string $selector |
||
276 | * |
||
277 | * @return false|SimpleXmlDomInterface |
||
278 | */ |
||
279 | 1 | public function findOneOrFalse(string $selector) |
|
289 | |||
290 | /** |
||
291 | * @param string $content |
||
292 | * @param bool $multiDecodeNewHtmlEntity |
||
293 | * |
||
294 | * @return string |
||
295 | */ |
||
296 | public function fixHtmlOutput(string $content, bool $multiDecodeNewHtmlEntity = false): string |
||
302 | |||
303 | /** |
||
304 | * Return elements by ".class". |
||
305 | * |
||
306 | * @param string $class |
||
307 | * |
||
308 | * @return SimpleXmlDomInterface[]|SimpleXmlDomNodeInterface<SimpleXmlDomInterface> |
||
309 | */ |
||
310 | public function getElementByClass(string $class): SimpleXmlDomNodeInterface |
||
314 | |||
315 | /** |
||
316 | * Return element by #id. |
||
317 | * |
||
318 | * @param string $id |
||
319 | * |
||
320 | * @return SimpleXmlDomInterface |
||
321 | */ |
||
322 | public function getElementById(string $id): SimpleXmlDomInterface |
||
326 | |||
327 | /** |
||
328 | * Return element by tag name. |
||
329 | * |
||
330 | * @param string $name |
||
331 | * |
||
332 | * @return SimpleXmlDomInterface |
||
333 | */ |
||
334 | public function getElementByTagName(string $name): SimpleXmlDomInterface |
||
344 | |||
345 | /** |
||
346 | * Returns elements by "#id". |
||
347 | * |
||
348 | * @param string $id |
||
349 | * @param int|null $idx |
||
350 | * |
||
351 | * @return SimpleXmlDomInterface|SimpleXmlDomInterface[]|SimpleXmlDomNodeInterface<SimpleXmlDomInterface> |
||
352 | */ |
||
353 | public function getElementsById(string $id, $idx = null) |
||
357 | |||
358 | /** |
||
359 | * Returns elements by tag name. |
||
360 | * |
||
361 | * @param string $name |
||
362 | * @param int|null $idx |
||
363 | * |
||
364 | * @return SimpleXmlDomInterface|SimpleXmlDomInterface[]|SimpleXmlDomNodeInterface<SimpleXmlDomInterface> |
||
365 | */ |
||
366 | View Code Duplication | public function getElementsByTagName(string $name, $idx = null) |
|
393 | |||
394 | /** |
||
395 | * Get dom node's outer html. |
||
396 | * |
||
397 | * @param bool $multiDecodeNewHtmlEntity |
||
398 | * |
||
399 | * @return string |
||
400 | */ |
||
401 | public function html(bool $multiDecodeNewHtmlEntity = false): string |
||
415 | |||
416 | /** |
||
417 | * Load HTML from string. |
||
418 | * |
||
419 | * @param string $html |
||
420 | * @param int|null $libXMLExtraOptions |
||
421 | * |
||
422 | * @return self |
||
423 | */ |
||
424 | public function loadHtml(string $html, $libXMLExtraOptions = null): DomParserInterface |
||
430 | |||
431 | /** |
||
432 | * Load HTML from file. |
||
433 | * |
||
434 | * @param string $filePath |
||
435 | * @param int|null $libXMLExtraOptions |
||
436 | * |
||
437 | * @throws \RuntimeException |
||
438 | * |
||
439 | * @return XmlDomParser |
||
440 | */ |
||
441 | View Code Duplication | public function loadHtmlFile(string $filePath, $libXMLExtraOptions = null): DomParserInterface |
|
468 | |||
469 | /** |
||
470 | * @param string $selector |
||
471 | * @param int $idx |
||
472 | * |
||
473 | * @return SimpleXmlDomInterface|SimpleXmlDomInterface[]|SimpleXmlDomNodeInterface<SimpleXmlDomInterface> |
||
474 | */ |
||
475 | public function __invoke($selector, $idx = null) |
||
479 | |||
480 | /** |
||
481 | * Load XML from string. |
||
482 | * |
||
483 | * @param string $xml |
||
484 | * @param int|null $libXMLExtraOptions |
||
485 | * |
||
486 | * @return XmlDomParser |
||
487 | */ |
||
488 | 3 | public function loadXml(string $xml, $libXMLExtraOptions = null): self |
|
494 | |||
495 | /** |
||
496 | * Load XML from file. |
||
497 | * |
||
498 | * @param string $filePath |
||
499 | * @param int|null $libXMLExtraOptions |
||
500 | * |
||
501 | * @throws \RuntimeException |
||
502 | * |
||
503 | * @return XmlDomParser |
||
504 | */ |
||
505 | 2 | View Code Duplication | public function loadXmlFile(string $filePath, $libXMLExtraOptions = null): self |
532 | |||
533 | /** |
||
534 | * @param callable $callback |
||
535 | * @param \DOMNode|null $domNode |
||
536 | * |
||
537 | * @return void |
||
538 | */ |
||
539 | 1 | public function replaceTextWithCallback($callback, \DOMNode $domNode = null) |
|
571 | } |
||
572 |
If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.
Let’s take a look at an example:
Our function
my_function
expects aPost
object, and outputs the author of the post. The base classPost
returns a simple string and outputting a simple string will work just fine. However, the child classBlogPost
which is a sub-type ofPost
instead decided to return anobject
, and is therefore violating the SOLID principles. If aBlogPost
were passed tomy_function
, PHP would not complain, but ultimately fail when executing thestrtoupper
call in its body.