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 RegularParser 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 RegularParser, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
12 | final class RegularParser implements ParserInterface |
||
13 | { |
||
14 | private $lexerRegex; |
||
15 | private $tokens; |
||
16 | private $tokensCount; |
||
17 | private $position; |
||
18 | private $backtracks; |
||
19 | |||
20 | const TOKEN_OPEN = 1; |
||
21 | const TOKEN_CLOSE = 2; |
||
22 | const TOKEN_MARKER = 3; |
||
23 | const TOKEN_SEPARATOR = 4; |
||
24 | const TOKEN_DELIMITER = 5; |
||
25 | const TOKEN_STRING = 6; |
||
26 | const TOKEN_WS = 7; |
||
27 | |||
28 | 4 | public function __construct(SyntaxInterface $syntax = null) |
|
32 | |||
33 | /** |
||
34 | * @param string $text |
||
35 | * |
||
36 | * @return ParsedShortcode[] |
||
37 | */ |
||
38 | 40 | public function parse($text) |
|
59 | |||
60 | 32 | private function getObject($name, $parameters, $bbCode, $offset, $content, $text) |
|
64 | |||
65 | /* --- RULES ----------------------------------------------------------- */ |
||
66 | |||
67 | 39 | private function shortcode(array &$names) |
|
68 | { |
||
69 | 39 | $name = null; |
|
70 | 39 | $offset = null; |
|
71 | |||
72 | $setName = function(array $token) use(&$name) { $name = $token[1]; }; |
||
73 | $setOffset = function(array $token) use(&$offset) { $offset = $token[2]; }; |
||
74 | |||
75 | 39 | if(!$this->match(self::TOKEN_OPEN, $setOffset, true)) { return false; } |
|
76 | 39 | if(!$this->match(self::TOKEN_STRING, $setName, false)) { return false; } |
|
77 | 36 | if($this->lookahead(self::TOKEN_STRING, null)) { return false; } |
|
78 | 36 | if(!preg_match_all('/^[a-zA-Z0-9-]+$/', $name, $matches)) { return false; } |
|
79 | 35 | $this->match(self::TOKEN_WS); |
|
80 | 35 | if(false === ($bbCode = $this->bbCode())) { return false; } |
|
81 | 35 | if(false === ($parameters = $this->parameters())) { return false; } |
|
82 | |||
83 | // self-closing |
||
84 | 33 | if($this->match(self::TOKEN_MARKER, null, true)) { |
|
85 | 12 | if(!$this->match(self::TOKEN_CLOSE)) { return false; } |
|
86 | |||
87 | 11 | return array($this->getObject($name, $parameters, $bbCode, $offset, null, $this->getBacktrack())); |
|
88 | } |
||
89 | |||
90 | // just-closed or with-content |
||
91 | 23 | if(!$this->match(self::TOKEN_CLOSE)) { return false; } |
|
92 | 23 | $this->beginBacktrack(); |
|
93 | 23 | $names[] = $name; |
|
94 | 23 | list($content, $shortcodes, $closingName) = $this->content($names); |
|
95 | 23 | if(null !== $closingName && $closingName !== $name) { |
|
96 | 1 | array_pop($names); |
|
97 | 1 | array_pop($this->backtracks); |
|
98 | 1 | array_pop($this->backtracks); |
|
99 | |||
100 | 1 | return $closingName; |
|
101 | } |
||
102 | 23 | if(false === $content || $closingName !== $name) { |
|
103 | 14 | $this->backtrack(false); |
|
104 | 14 | $text = $this->backtrack(false); |
|
105 | |||
106 | 14 | return array_merge(array($this->getObject($name, $parameters, $bbCode, $offset, null, $text)), $shortcodes); |
|
107 | } |
||
108 | 10 | $content = $this->getBacktrack(); |
|
109 | 10 | if(!$this->close($names)) { return false; } |
|
110 | |||
111 | 10 | return array($this->getObject($name, $parameters, $bbCode, $offset, $content, $this->getBacktrack())); |
|
112 | } |
||
113 | |||
114 | 23 | private function content(array &$names) |
|
115 | { |
||
116 | 23 | $content = null; |
|
117 | 23 | $shortcodes = array(); |
|
118 | 23 | $closingName = null; |
|
119 | $appendContent = function(array $token) use(&$content) { $content .= $token[1]; }; |
||
120 | |||
121 | 23 | while($this->position < $this->tokensCount) { |
|
122 | 15 | while($this->match(array(self::TOKEN_STRING, self::TOKEN_WS), $appendContent)) { |
|
123 | 14 | continue; |
|
124 | } |
||
125 | |||
126 | 15 | $this->beginBacktrack(); |
|
127 | 15 | $matchedShortcodes = $this->shortcode($names); |
|
128 | 15 | if(is_string($matchedShortcodes)) { |
|
129 | 1 | $closingName = $matchedShortcodes; |
|
130 | 1 | break; |
|
131 | } |
||
132 | 15 | if(false !== $matchedShortcodes) { |
|
133 | 6 | $shortcodes = array_merge($shortcodes, $matchedShortcodes); |
|
134 | 6 | continue; |
|
135 | } |
||
136 | 13 | $this->backtrack(); |
|
137 | |||
138 | 13 | $this->beginBacktrack(); |
|
139 | 13 | if(false !== ($closingName = $this->close($names))) { |
|
140 | 10 | if(null === $content) { $content = ''; } |
|
141 | 10 | $this->backtrack(); |
|
142 | 10 | $shortcodes = array(); |
|
143 | 10 | break; |
|
144 | } |
||
145 | 4 | $closingName = null; |
|
146 | 4 | $this->backtrack(); |
|
147 | 4 | if($this->position < $this->tokensCount) { |
|
148 | 1 | $shortcodes = array(); |
|
149 | 1 | break; |
|
150 | } |
||
151 | |||
152 | 3 | $this->match(null, $appendContent); |
|
153 | 3 | } |
|
154 | |||
155 | 23 | return array($this->position < $this->tokensCount ? $content : false, $shortcodes, $closingName); |
|
156 | } |
||
157 | |||
158 | 13 | private function close(array &$names) |
|
170 | |||
171 | 35 | private function bbCode() |
|
175 | |||
176 | 35 | private function parameters() |
|
196 | |||
197 | 21 | private function value() |
|
212 | |||
213 | /* --- PARSER ---------------------------------------------------------- */ |
||
214 | |||
215 | /** |
||
216 | * This method is used only for debugging purposes. DO NOT DELETE. |
||
217 | * |
||
218 | * @return array |
||
219 | */ |
||
220 | private function listBacktracks() |
||
226 | |||
227 | 39 | private function beginBacktrack() |
|
231 | |||
232 | private function getBacktrack() |
||
237 | |||
238 | 23 | private function backtrack($modifyPosition = true) |
|
256 | |||
257 | 39 | private function lookahead($type, $callback = null) |
|
274 | |||
275 | 39 | private function match($type, $callbacks = null, $ws = false) |
|
299 | |||
300 | /* --- LEXER ----------------------------------------------------------- */ |
||
301 | |||
302 | 40 | private function tokenize($text) |
|
325 | |||
326 | private function getTokenizerRegex(SyntaxInterface $syntax) |
||
344 | } |
||
345 |
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.