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 BinaryTreeAbstract 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 BinaryTreeAbstract, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
23 | abstract class BinaryTreeAbstract implements TreeInterface { |
||
24 | use CountTrait; |
||
25 | protected $root; |
||
26 | protected $size = 0; |
||
27 | |||
28 | abstract public function createNode($key, $data, $parent = null, $left = null, $right = null); |
||
29 | /** |
||
30 | * Checks if the tree is empty. |
||
31 | * |
||
32 | * @return boolean true if is empty, else false. |
||
33 | */ |
||
34 | public function empty() : bool { |
||
37 | |||
38 | /** |
||
39 | * Returns the tree size. |
||
40 | * |
||
41 | * @return int the length |
||
42 | */ |
||
43 | public function size() : int { |
||
46 | |||
47 | /** |
||
48 | * Inserts data in the correct position. |
||
49 | * |
||
50 | * @param integer|string $key the key used to store. |
||
51 | * @param mixed $data the data to store. |
||
52 | * @param bool $update if false the node isn't updated |
||
53 | * else if the key matches is updated. |
||
54 | */ |
||
55 | public function put($key, $data, $update = false) { |
||
91 | |||
92 | /** |
||
93 | * Creates a new node or updates it if already exists. |
||
94 | * |
||
95 | * @param int|string $key the key. |
||
96 | * @param mixed $data the data to be stored. |
||
97 | */ |
||
98 | public function putOrUpdate($key, $data) { |
||
101 | |||
102 | /** |
||
103 | * Retrieve the data stored in the tree. |
||
104 | * |
||
105 | * @param int|string $key the key to identify the data. |
||
106 | * @return mixed |
||
107 | */ |
||
108 | public function get($key) { |
||
126 | |||
127 | /** |
||
128 | * Returns the root node. |
||
129 | * |
||
130 | * @return DataStructures\Trees\Nodes\BinaryNodeInterface|null the root node. |
||
131 | */ |
||
132 | public function getRoot() { |
||
135 | |||
136 | /** |
||
137 | * Looks for the node with the given key. |
||
138 | * |
||
139 | * @param int|string $key the key used to look for. |
||
140 | * @return bool true if was found. |
||
141 | */ |
||
142 | public function exists($key) : bool { |
||
165 | |||
166 | /** |
||
167 | * Method that retrieves true if found a node with the specified key. |
||
168 | * It's the recursive version of exists method and it's used internally |
||
169 | * for traverse through a root node. |
||
170 | * |
||
171 | * @param DataStructures\Trees\Nodes\BinaryNodeInterface|null $node the root node. |
||
172 | * @param int|string $key the key used to look for. |
||
173 | * @return bool true if exists a node with that key. |
||
174 | */ |
||
175 | private function _exists($node, $key) : bool { |
||
188 | |||
189 | public function floor($key){} |
||
191 | |||
192 | /** |
||
193 | * Gets the node with the minimum key. The most left and more bottom. |
||
194 | * |
||
195 | * @return DataStructures\Trees\Nodes\BinaryNodeInterface|null the minum node or |
||
196 | * null if the tree is empty. |
||
197 | */ |
||
198 | public function min() { |
||
214 | |||
215 | /** |
||
216 | * Gets the node with the maximum key. The most right and more bottom. |
||
217 | * |
||
218 | * @return DataStructures\Trees\Nodes\BinaryNodeInterface|null the maximum node or |
||
219 | * null if the tree is empty. |
||
220 | */ |
||
221 | public function max() { |
||
237 | |||
238 | /** |
||
239 | * Returns the minimum node from a given node in position X. |
||
240 | * |
||
241 | * @param DataStructures\Trees\Nodes\BinaryNodeInterface $node the start point. |
||
242 | * @return DataStructures\Trees\Nodes\BinaryNodeInterface|null the minimum node. |
||
243 | */ |
||
244 | View Code Duplication | protected function getMinNode(BinaryNodeInterface $node = null) { |
|
255 | |||
256 | /** |
||
257 | * Returns the maximum node from a given node in position X. |
||
258 | * |
||
259 | * @param DataStructures\Trees\Nodes\BinaryNodeInterface $node the start point. |
||
260 | * @return DataStructures\Trees\Nodes\BinaryNodeInterface|null the maximum node. |
||
261 | */ |
||
262 | View Code Duplication | protected function getMaxNode(BinaryNodeInterface $node = null) { |
|
273 | |||
274 | /** |
||
275 | * Deletes the node with the minimum key and returns it. The most left and more bottom. |
||
276 | * |
||
277 | * @param DataStructures\Trees\Nodes\BinaryNodeInterface|null if null takes the root. |
||
278 | * @return DataStructures\Trees\Nodes\BinaryNodeInterface|null the minimum node or |
||
279 | * null if the tree is empty. |
||
280 | */ |
||
281 | View Code Duplication | public function deleteMin(BinaryNodeInterface $node = null) { |
|
289 | |||
290 | /** |
||
291 | * Deletes the node with the maximum key and returns it. The most right and more bottom. |
||
292 | * |
||
293 | * @param DataStructures\Trees\Nodes\BinaryNodeInterface|null if null takes the root. |
||
294 | * @return DataStructures\Trees\Nodes\BinaryNodeInterface|null the maximum node or |
||
295 | * null if the tree is empty. |
||
296 | */ |
||
297 | View Code Duplication | public function deleteMax(BinaryNodeInterface $node = null) { |
|
305 | |||
306 | /** |
||
307 | * Deletes the node with the maximum key and returns it. The most right and more bottom. |
||
308 | * |
||
309 | * @param DataStructures\Trees\Nodes\BinaryNodeInterface|null if null takes the root. |
||
310 | * @return DataStructures\Trees\Nodes\BinaryNodeInterface|null the maximum node or |
||
311 | * null if the tree is empty. |
||
312 | */ |
||
313 | public function delete($key) { |
||
322 | |||
323 | /** |
||
324 | * Deletes the selected node if is not null and returns the node |
||
325 | * that replaces the deleted node. Also decrease the size of tree. |
||
326 | * |
||
327 | * @param DataStructures\Trees\Nodes\BinaryNodeInterface|null The node to be deleted. |
||
328 | * @return the node that replaces the deleted. |
||
329 | */ |
||
330 | protected function _delete(BinaryNodeInterface &$node) { |
||
357 | |||
358 | /** |
||
359 | * Replaces the node n to remove a new one k and links k with the parent |
||
360 | * of n. |
||
361 | * |
||
362 | * @param DataStructures\Trees\Nodes\BinaryNodeInterface $nodeToReplace the node to remove. |
||
363 | * @param DataStructures\Trees\Nodes\BinaryNodeInterface|null $newNode the newNode |
||
364 | * to link with the $nodeToReplace parent. |
||
365 | * @return DataStructures\Trees\Nodes\BinaryNodeInterface the new linked node. |
||
366 | */ |
||
367 | protected function replace(&$nodeToReplace, &$newNode) { |
||
382 | |||
383 | /** |
||
384 | * Retrieves the node with the specified key. |
||
385 | * |
||
386 | * @param int|string $key the key used to store. |
||
387 | * @return DataStructures\Trees\Nodes\BinaryNodeInterface|null the node or null. |
||
388 | */ |
||
389 | public function search($key) { |
||
411 | |||
412 | /** |
||
413 | * Returns true if is leaf the node. |
||
414 | * |
||
415 | * @param DataStructures\Trees\Nodes\BinaryNodeInterface|null $node default to null. |
||
416 | * @return true if is leaf the node, is not null and their subtrees has no |
||
417 | * pointers to successors. |
||
418 | */ |
||
419 | public function isLeaf($node) : bool { // BinaryTreeNode |
||
422 | |||
423 | /** |
||
424 | * Checks if a node is root. New nodes that does not point to any other node |
||
425 | * also are called a root node. |
||
426 | * |
||
427 | * @param DataStructures\Trees\Nodes\BinaryNodeInterface|null $node default to null. |
||
428 | * @return true if is root the node, is not null and their subtrees has no |
||
429 | * pointers to successors. |
||
430 | */ |
||
431 | public function isRoot($node) : bool { |
||
434 | |||
435 | /** |
||
436 | * Traverse in preorder. This is: first visit the root, then |
||
437 | * the left subtree and finally the right subtree. |
||
438 | * |
||
439 | * @param Callable|null $callback the callback function to apply to each |
||
440 | * node. |
||
441 | */ |
||
442 | public function preorder(Callable $callback = null) { |
||
445 | |||
446 | /** |
||
447 | * Private preorder traverse method that is recursive and is called by |
||
448 | * the public preorder method. |
||
449 | * |
||
450 | * @param DataStructures\Trees\Nodes\BinaryNodeInterface|null $node. |
||
451 | * @param Callable|null $callback the callback function to apply to each |
||
452 | * node. |
||
453 | */ |
||
454 | View Code Duplication | private function _preorder($node, Callable $callback = null) { |
|
464 | |||
465 | /** |
||
466 | * Traverse in inorder. This is: first visit the left subtree, |
||
467 | * then the root and finally the right subtree. |
||
468 | * |
||
469 | * @param Callable|null $callback the callback function to apply to each |
||
470 | * node. |
||
471 | */ |
||
472 | public function inorder(Callable $callback = null) { |
||
475 | |||
476 | /** |
||
477 | * Private inorder traverse method that is recursive and is called by |
||
478 | * the public inorder method. |
||
479 | * |
||
480 | * @param DataStructures\Trees\Nodes\BinaryNodeInterface|null $node. |
||
481 | * @param Callable|null $callback the callback function to apply to each |
||
482 | * node. |
||
483 | */ |
||
484 | View Code Duplication | private function _inorder($node, Callable $callback = null) { |
|
495 | |||
496 | /** |
||
497 | * Traverse in postorder. This is: first visit the left subtree, |
||
498 | * then the right subtree and finally the root. |
||
499 | * |
||
500 | * @param Callable|null $callback the callback function to apply to each |
||
501 | * node. |
||
502 | */ |
||
503 | public function postorder(Callable $callback = null) { |
||
506 | |||
507 | /** |
||
508 | * Private postorder traverse method that is recursive and is called by |
||
509 | * the public postorder method. |
||
510 | * |
||
511 | * @param DataStructures\Trees\Nodes\BinaryNodeInterface|null $node. |
||
512 | * @param Callable|null $callback the callback function to apply to each |
||
513 | * node. |
||
514 | */ |
||
515 | View Code Duplication | private function _postorder($node, Callable $callback = null) { |
|
525 | } |
Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.
The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.
This check looks for comments that seem to be mostly valid code and reports them.