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 LogicalFilter 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 LogicalFilter, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
29 | class LogicalFilter implements \JsonSerializable |
||
30 | { |
||
31 | /** @var AndRule $rules */ |
||
32 | protected $rules; |
||
33 | |||
34 | /** @var Filterer $default_filterer */ |
||
35 | protected $default_filterer; |
||
36 | |||
37 | /** @var array $options */ |
||
38 | protected $options = []; |
||
39 | |||
40 | /** @var array $default_options */ |
||
41 | protected static $default_options = [ |
||
42 | 'in.normalization_threshold' => 0, |
||
43 | ]; |
||
44 | |||
45 | /** |
||
46 | * Creates a filter. You can provide a description of rules as in |
||
47 | * addRules() as paramater. |
||
48 | * |
||
49 | * @param array $rules |
||
50 | * @param Filterer $default_filterer |
||
51 | * |
||
52 | * @see self::addRules |
||
53 | */ |
||
54 | 145 | public function __construct($rules=[], Filterer $default_filterer=null, array $options=[]) |
|
78 | |||
79 | /** |
||
80 | */ |
||
81 | 9 | protected function getDefaultFilterer() |
|
89 | |||
90 | /** |
||
91 | */ |
||
92 | 1 | public static function setDefaultOptions(array $options) |
|
100 | |||
101 | /** |
||
102 | * @return array |
||
103 | */ |
||
104 | 51 | public static function getDefaultOptions() |
|
108 | |||
109 | /** |
||
110 | * @return array |
||
111 | */ |
||
112 | 135 | public function getOptions() |
|
121 | |||
122 | /** |
||
123 | * This method parses different ways to define the rules of a LogicalFilter. |
||
124 | * + You can add N already instanciated Rules. |
||
125 | * + You can provide 3 arguments: $field, $operator, $value |
||
126 | * + You can provide a tree of rules: |
||
127 | * [ |
||
128 | * 'or', |
||
129 | * [ |
||
130 | * 'and', |
||
131 | * ['field_5', 'above', 'a'], |
||
132 | * ['field_5', 'below', 'a'], |
||
133 | * ], |
||
134 | * ['field_6', 'equal', 'b'], |
||
135 | * ] |
||
136 | * |
||
137 | * @param string $operation and | or |
||
138 | * @param array $rules_description Rules description |
||
139 | * @return LogicalFilter $this |
||
140 | */ |
||
141 | 141 | protected function addRules( $operation, array $rules_description ) |
|
226 | |||
227 | /** |
||
228 | * Add one rule object to the filter |
||
229 | * |
||
230 | * @param AbstractRule $rule |
||
231 | * @param string $operation |
||
232 | * |
||
233 | * @return $this |
||
234 | */ |
||
235 | 137 | protected function addRule( AbstractRule $rule, $operation=AndRule::operator ) |
|
271 | |||
272 | /** |
||
273 | * Recursion auxiliary of addCompositeRule. |
||
274 | * |
||
275 | * @param array $rules_composition The description of the |
||
276 | * rules to add. |
||
277 | * @param AbstractOperationRule $recursion_position The position in the |
||
278 | * tree where rules must |
||
279 | * be added. |
||
280 | * |
||
281 | * @return $this |
||
282 | */ |
||
283 | 132 | protected function addCompositeRule_recursion( |
|
415 | |||
416 | /** |
||
417 | * This method parses different ways to define the rules of a LogicalFilter |
||
418 | * and add them as a new And part of the filter. |
||
419 | * + You can add N already instanciated Rules. |
||
420 | * + You can provide 3 arguments: $field, $operator, $value |
||
421 | * + You can provide a tree of rules: |
||
422 | * [ |
||
423 | * 'or', |
||
424 | * [ |
||
425 | * 'and', |
||
426 | * ['field_5', 'above', 'a'], |
||
427 | * ['field_5', 'below', 'a'], |
||
428 | * ], |
||
429 | * ['field_6', 'equal', 'b'], |
||
430 | * ] |
||
431 | * |
||
432 | * @param mixed The descriptions of the rules to add |
||
433 | * @return $this |
||
434 | * |
||
435 | * @todo remove the _ for PHP 7 |
||
436 | */ |
||
437 | 138 | public function and_() |
|
442 | |||
443 | /** |
||
444 | * This method parses different ways to define the rules of a LogicalFilter |
||
445 | * and add them as a new Or part of the filter. |
||
446 | * + You can add N already instanciated Rules. |
||
447 | * + You can provide 3 arguments: $field, $operator, $value |
||
448 | * + You can provide a tree of rules: |
||
449 | * [ |
||
450 | * 'or', |
||
451 | * [ |
||
452 | * 'and', |
||
453 | * ['field_5', 'above', 'a'], |
||
454 | * ['field_5', 'below', 'a'], |
||
455 | * ], |
||
456 | * ['field_6', 'equal', 'b'], |
||
457 | * ] |
||
458 | * |
||
459 | * @param mixed The descriptions of the rules to add |
||
460 | * @return $this |
||
461 | * |
||
462 | * @todo |
||
463 | * @todo remove the _ for PHP 7 |
||
464 | */ |
||
465 | 7 | public function or_() |
|
470 | |||
471 | /** |
||
472 | * @deprecated |
||
473 | */ |
||
474 | 1 | public function matches($rules_to_match) |
|
478 | |||
479 | /** |
||
480 | * Checks that a filter matches another one. |
||
481 | * |
||
482 | * @param array|AbstractRule $rules_to_match |
||
483 | * |
||
484 | * @return bool Whether or not this combination of filters has |
||
485 | * potential solutions |
||
486 | */ |
||
487 | 1 | public function hasSolutionIf($rules_to_match) |
|
495 | |||
496 | /** |
||
497 | * Retrieve all the rules. |
||
498 | * |
||
499 | * @param bool $copy By default copy the rule tree to avoid side effects. |
||
500 | * |
||
501 | * @return AbstractRule The tree of rules |
||
502 | */ |
||
503 | 56 | public function getRules($copy = true) |
|
507 | |||
508 | /** |
||
509 | * Remove any constraint being a duplicate of another one. |
||
510 | * |
||
511 | * @param array $options stop_after | stop_before | |
||
512 | * @return $this |
||
513 | */ |
||
514 | 93 | public function simplify($options=[]) |
|
526 | |||
527 | /** |
||
528 | * Checks if there is at least on set of conditions which is not |
||
529 | * contradictory. |
||
530 | * |
||
531 | * Checking if a filter has solutions require to simplify it first. |
||
532 | * To let the control on the balance between readability and |
||
533 | * performances, the required simplification can be saved or not |
||
534 | * depending on the $save_simplification parameter. |
||
535 | * |
||
536 | * @param $save_simplification |
||
537 | * |
||
538 | * @return bool |
||
539 | */ |
||
540 | 8 | public function hasSolution($save_simplification=true) |
|
553 | |||
554 | /** |
||
555 | * Returns an array describing the rule tree of the Filter. |
||
556 | * |
||
557 | * @param array $options |
||
558 | * |
||
559 | * @return array A description of the rules. |
||
560 | */ |
||
561 | 95 | public function toArray(array $options=[]) |
|
565 | |||
566 | /** |
||
567 | * Returns an array describing the rule tree of the Filter. |
||
568 | * |
||
569 | * @param $debug Provides a source oriented dump. |
||
570 | * |
||
571 | * @return array A description of the rules. |
||
572 | */ |
||
573 | 4 | public function toString(array $options=[]) |
|
577 | |||
578 | /** |
||
579 | * Returns a unique id corresponding to the set of rules of the filter |
||
580 | * |
||
581 | * @return string The unique semantic id |
||
582 | */ |
||
583 | 1 | public function getSemanticId() |
|
587 | |||
588 | /** |
||
589 | * For implementing JsonSerializable interface. |
||
590 | * |
||
591 | * @see https://secure.php.net/manual/en/jsonserializable.jsonserialize.php |
||
592 | */ |
||
593 | 1 | public function jsonSerialize() |
|
597 | |||
598 | /** |
||
599 | * @return string |
||
600 | */ |
||
601 | 4 | public function __toString() |
|
605 | |||
606 | /** |
||
607 | * @see https://secure.php.net/manual/en/language.oop5.magic.php#object.invoke |
||
608 | * @param mixed $row |
||
609 | * @return bool |
||
610 | */ |
||
611 | 3 | public function __invoke($row, $key=null) |
|
615 | |||
616 | /** |
||
617 | * Removes all the defined rules. |
||
618 | * |
||
619 | * @return $this |
||
620 | */ |
||
621 | 2 | public function flushRules() |
|
626 | |||
627 | /** |
||
628 | * @param array|callable Associative array of renamings or callable |
||
629 | * that would rename the fields. |
||
630 | * |
||
631 | * @return string $this |
||
632 | */ |
||
633 | 1 | public function renameFields($renamings) |
|
644 | |||
645 | /** |
||
646 | * @param array|callable Associative array of renamings or callable |
||
647 | * that would rename the fields. |
||
648 | * |
||
649 | * @return string $this |
||
650 | */ |
||
651 | 10 | public function removeRules($filter) |
|
687 | |||
688 | /** |
||
689 | * @param array|callable Associative array of renamings or callable |
||
690 | * that would rename the fields. |
||
691 | * |
||
692 | * @return array The rules matching the filter |
||
693 | * @return array $options debug | leaves_only | clean_empty_branches |
||
694 | * |
||
695 | * |
||
696 | * @todo Merge with rules |
||
697 | */ |
||
698 | 4 | public function keepLeafRulesMatching($filter=[], array $options=[]) |
|
737 | |||
738 | /** |
||
739 | * @param array|callable Associative array of renamings or callable |
||
740 | * that would rename the fields. |
||
741 | * |
||
742 | * @return array The rules matching the filter |
||
743 | * |
||
744 | * |
||
745 | * @todo Merge with rules |
||
746 | */ |
||
747 | 3 | public function listLeafRulesMatching($filter=[]) |
|
780 | |||
781 | /** |
||
782 | * @param array|callable Associative array of renamings or callable |
||
783 | * that would rename the fields. |
||
784 | * |
||
785 | * @return array The rules matching the filter |
||
786 | * |
||
787 | * |
||
788 | * @todo Make it available on AbstractRule also |
||
789 | */ |
||
790 | 2 | public function onEachRule($filter=[], $options) |
|
814 | |||
815 | /** |
||
816 | */ |
||
817 | 1 | public function onEachCase(callable $action) |
|
839 | |||
840 | /** |
||
841 | * Clone the current object and its rules. |
||
842 | * |
||
843 | * @return LogicalFilter A copy of the current instance with a copied ruletree |
||
844 | */ |
||
845 | 7 | public function copy() |
|
849 | |||
850 | /** |
||
851 | * Make a deep copy of the rules |
||
852 | */ |
||
853 | 7 | public function __clone() |
|
859 | |||
860 | /** |
||
861 | * Copy the current instance into the variable given as parameter |
||
862 | * and returns the copy. |
||
863 | * |
||
864 | * @return LogicalFilter |
||
865 | */ |
||
866 | 3 | public function saveAs( &$variable ) |
|
870 | |||
871 | /** |
||
872 | * Copy the current instance into the variable given as parameter |
||
873 | * and returns the copied instance. |
||
874 | * |
||
875 | * @return LogicalFilter |
||
876 | */ |
||
877 | 1 | public function saveCopyAs( &$copied_variable ) |
|
882 | |||
883 | /** |
||
884 | * @param bool $exit=false |
||
885 | * @param array $options + callstack_depth=2 The level of the caller to dump |
||
886 | * + mode='string' in 'export' | 'dump' | 'string' |
||
887 | * |
||
888 | * @return $this |
||
889 | */ |
||
890 | 4 | public function dump($exit=false, array $options=[]) |
|
941 | |||
942 | /** |
||
943 | * Applies the current instance to a set of data. |
||
944 | * |
||
945 | * @param mixed $data_to_filter |
||
946 | * @param Filterer|callable|null $filterer |
||
947 | * |
||
948 | * @return mixed The filtered data |
||
949 | */ |
||
950 | 5 | public function applyOn($data_to_filter, $action_on_matches=null, $filterer=null) |
|
973 | |||
974 | /** |
||
975 | * Applies the current instance to a value (and its index optionnally). |
||
976 | * |
||
977 | * @param mixed $value_to_check |
||
978 | * @param scalar $index |
||
979 | * @param Filterer|callable|null $filterer |
||
980 | * |
||
981 | * @return AbstractRule|false|true + False if the filter doesn't validates |
||
982 | * + Null if the target has no sens (operation filtered by field for example) |
||
983 | * + A rule tree containing the first matching case if there is one. |
||
984 | */ |
||
985 | 4 | public function validates($value_to_check, $key_to_check=null, $filterer=null) |
|
1002 | |||
1003 | /**/ |
||
1004 | } |
||
1005 |
This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.
Consider making the comparison explicit by using
empty(..)
or! empty(...)
instead.