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 AST 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 AST, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
16 | class AST |
||
17 | { |
||
18 | /** |
||
19 | * @var Node[] |
||
20 | */ |
||
21 | private $nodes = []; |
||
22 | |||
23 | public function __construct(array $nodes) |
||
27 | |||
28 | /** |
||
29 | * Converts the supplied php parser nodes to an equivalent |
||
30 | * expression tree. |
||
31 | * |
||
32 | * @param Node[] $nodes |
||
33 | * |
||
34 | * @return Expression[] |
||
35 | */ |
||
36 | public static function convert(array $nodes) |
||
40 | |||
41 | /** |
||
42 | * Parses the nodes into the equivalent expression tree |
||
43 | * |
||
44 | * @return Expression[] |
||
45 | */ |
||
46 | public function getExpressions() |
||
50 | |||
51 | /** |
||
52 | * @param Node[] $nodes |
||
53 | * |
||
54 | * @return Expression[] |
||
55 | */ |
||
56 | private function parseNodes(array $nodes) |
||
65 | |||
66 | /** |
||
67 | * @param Node $node |
||
68 | * |
||
69 | * @throws \Pinq\Parsing\ASTException |
||
70 | * @return Expression |
||
71 | */ |
||
72 | protected function parseNode(Node $node) |
||
91 | |||
92 | /** |
||
93 | * @param $node |
||
94 | * |
||
95 | * @return Expression |
||
96 | */ |
||
97 | final public function parseNameNode($node) |
||
107 | |||
108 | protected function parseAbsoluteName(Node\Name $node) |
||
112 | |||
113 | private function parseParameterNode(Node\Param $node) |
||
132 | |||
133 | private function parseArgumentNode(Node\Arg $node) |
||
140 | |||
141 | // <editor-fold defaultstate="collapsed" desc="Expression node parsers"> |
||
142 | |||
143 | public function parseExpressionNode(Node\Expr $node) |
||
227 | |||
228 | private function parseArrayNode(Node\Expr\Array_ $node) |
||
243 | |||
244 | private function parseFunctionCallNode(Node\Expr\FuncCall $node) |
||
260 | |||
261 | private function parseTernaryNode(Node\Expr\Ternary $node) |
||
269 | |||
270 | private function parseClosureNode(Node\Expr\Closure $node) |
||
292 | |||
293 | private function parseScalarNode(Node\Scalar $node) |
||
311 | |||
312 | // </editor-fold> |
||
313 | |||
314 | // <editor-fold defaultstate="collapsed" desc="Statement node parsers"> |
||
315 | |||
316 | private function parseStatementNode(Node\Stmt $node) |
||
336 | |||
337 | private static $constructStructureMap = [ |
||
338 | 'Do' => ASTException::DO_WHILE_LOOP, |
||
339 | 'For' => ASTException::FOR_LOOP, |
||
340 | 'Foreach' => ASTException::FOREACH_LOOP, |
||
341 | 'Goto' => ASTException::GOTO_STATEMENT, |
||
342 | 'If' => ASTException::IF_STATEMENT, |
||
343 | 'Switch' => ASTException::SWITCH_STATEMENT, |
||
344 | 'TryCatch' => ASTException::TRY_CATCH_STATEMENT, |
||
345 | 'While' => ASTException::WHILE_LOOP |
||
346 | ]; |
||
347 | |||
348 | private function verifyNotControlStructure(Node\Stmt $node) |
||
359 | |||
360 | // </editor-fold> |
||
361 | |||
362 | // <editor-fold defaultstate="collapsed" desc="Operator node maps"> |
||
363 | |||
364 | private function parseOperatorNode(Node\Expr $node) |
||
406 | |||
407 | private static $unaryOperatorsMap = [ |
||
408 | 'BitwiseNot' => Operators\Unary::BITWISE_NOT, |
||
409 | 'BooleanNot' => Operators\Unary::NOT, |
||
410 | 'PostInc' => Operators\Unary::INCREMENT, |
||
411 | 'PostDec' => Operators\Unary::DECREMENT, |
||
412 | 'PreInc' => Operators\Unary::PRE_INCREMENT, |
||
413 | 'PreDec' => Operators\Unary::PRE_DECREMENT, |
||
414 | 'UnaryMinus' => Operators\Unary::NEGATION, |
||
415 | 'UnaryPlus' => Operators\Unary::PLUS |
||
416 | ]; |
||
417 | |||
418 | private static $castOperatorMap = [ |
||
419 | 'Cast_Array' => Operators\Cast::ARRAY_CAST, |
||
420 | 'Cast_Bool' => Operators\Cast::BOOLEAN, |
||
421 | 'Cast_Double' => Operators\Cast::DOUBLE, |
||
422 | 'Cast_Int' => Operators\Cast::INTEGER, |
||
423 | 'Cast_Object' => Operators\Cast::OBJECT, |
||
424 | 'Cast_String' => Operators\Cast::STRING |
||
425 | ]; |
||
426 | |||
427 | private static $binaryOperatorsMap = [ |
||
428 | 'BinaryOp_BitwiseAnd' => Operators\Binary::BITWISE_AND, |
||
429 | 'BinaryOp_BitwiseOr' => Operators\Binary::BITWISE_OR, |
||
430 | 'BinaryOp_BitwiseXor' => Operators\Binary::BITWISE_XOR, |
||
431 | 'BinaryOp_ShiftLeft' => Operators\Binary::SHIFT_LEFT, |
||
432 | 'BinaryOp_ShiftRight' => Operators\Binary::SHIFT_RIGHT, |
||
433 | 'BinaryOp_BooleanAnd' => Operators\Binary::LOGICAL_AND, |
||
434 | 'BinaryOp_BooleanOr' => Operators\Binary::LOGICAL_OR, |
||
435 | 'BinaryOp_LogicalAnd' => Operators\Binary::LOGICAL_AND, |
||
436 | 'BinaryOp_LogicalOr' => Operators\Binary::LOGICAL_OR, |
||
437 | 'BinaryOp_Plus' => Operators\Binary::ADDITION, |
||
438 | 'BinaryOp_Minus' => Operators\Binary::SUBTRACTION, |
||
439 | 'BinaryOp_Mul' => Operators\Binary::MULTIPLICATION, |
||
440 | 'BinaryOp_Div' => Operators\Binary::DIVISION, |
||
441 | 'BinaryOp_Mod' => Operators\Binary::MODULUS, |
||
442 | 'BinaryOp_Pow' => Operators\Binary::POWER, |
||
443 | 'BinaryOp_Concat' => Operators\Binary::CONCATENATION, |
||
444 | 'BinaryOp_Equal' => Operators\Binary::EQUALITY, |
||
445 | 'BinaryOp_Identical' => Operators\Binary::IDENTITY, |
||
446 | 'BinaryOp_NotEqual' => Operators\Binary::INEQUALITY, |
||
447 | 'BinaryOp_NotIdentical' => Operators\Binary::NOT_IDENTICAL, |
||
448 | 'BinaryOp_Smaller' => Operators\Binary::LESS_THAN, |
||
449 | 'BinaryOp_SmallerOrEqual' => Operators\Binary::LESS_THAN_OR_EQUAL_TO, |
||
450 | 'BinaryOp_Greater' => Operators\Binary::GREATER_THAN, |
||
451 | 'BinaryOp_GreaterOrEqual' => Operators\Binary::GREATER_THAN_OR_EQUAL_TO |
||
452 | ]; |
||
453 | |||
454 | private static $assignOperatorsMap = [ |
||
455 | 'Assign' => Operators\Assignment::EQUAL, |
||
456 | 'AssignRef' => Operators\Assignment::EQUAL_REFERENCE, |
||
457 | 'AssignOp_BitwiseAnd' => Operators\Assignment::BITWISE_AND, |
||
458 | 'AssignOp_BitwiseOr' => Operators\Assignment::BITWISE_OR, |
||
459 | 'AssignOp_BitwiseXor' => Operators\Assignment::BITWISE_XOR, |
||
460 | 'AssignOp_Concat' => Operators\Assignment::CONCATENATE, |
||
461 | 'AssignOp_Div' => Operators\Assignment::DIVISION, |
||
462 | 'AssignOp_Minus' => Operators\Assignment::SUBTRACTION, |
||
463 | 'AssignOp_Mod' => Operators\Assignment::MODULUS, |
||
464 | 'AssignOp_Mul' => Operators\Assignment::MULTIPLICATION, |
||
465 | 'AssignOp_Pow' => Operators\Assignment::POWER, |
||
466 | 'AssignOp_Plus' => Operators\Assignment::ADDITION, |
||
467 | 'AssignOp_ShiftLeft' => Operators\Assignment::SHIFT_LEFT, |
||
468 | 'AssignOp_ShiftRight' => Operators\Assignment::SHIFT_RIGHT |
||
469 | ]; |
||
470 | |||
471 | // </editor-fold> |
||
472 | } |
||
473 |
If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:
If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.