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.