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 absences_RightAct 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 absences_RightAct, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 31 | class absences_RightAct |
||
| 32 | { |
||
| 33 | private static $post; |
||
| 34 | |||
| 35 | |||
| 36 | |||
| 37 | /** |
||
| 38 | * Get date from field name |
||
| 39 | * @param string $name |
||
| 40 | * @return string |
||
| 41 | */ |
||
| 42 | private static function getDateValue($name) |
||
| 59 | |||
| 60 | |||
| 61 | /** |
||
| 62 | * |
||
| 63 | * @param string $datename name of datepicker |
||
| 64 | * @param string $timename Name of select contains times values |
||
| 65 | * @return string |
||
| 66 | */ |
||
| 67 | private static function getDateTimeValue($datename, $timename) |
||
| 96 | |||
| 97 | |||
| 98 | /** |
||
| 99 | * |
||
| 100 | * @param string $begin Field name |
||
| 101 | * @param string $end Field name |
||
| 102 | * @param string $title title for error message |
||
| 103 | * |
||
| 104 | * @return boolean |
||
| 105 | */ |
||
| 106 | private static function checkPeriod($begin, $end, $title) |
||
| 133 | |||
| 134 | /** |
||
| 135 | * Validate optional period |
||
| 136 | * |
||
| 137 | * @param string $begin Field name |
||
| 138 | * @param string $end Field name |
||
| 139 | * @param string $title title for error message |
||
| 140 | * |
||
| 141 | * @return boolean |
||
| 142 | */ |
||
| 143 | private static function checkOptionalPeriod($begin, $end, $title) |
||
| 153 | |||
| 154 | |||
| 155 | |||
| 156 | /** |
||
| 157 | * Test posted values before saving |
||
| 158 | * @return bool |
||
| 159 | */ |
||
| 160 | protected static function checkPost() |
||
| 303 | |||
| 304 | |||
| 305 | /** |
||
| 306 | * Si le droit est fixe+modifie ou si le droit deviens fixe |
||
| 307 | * @param absences_Right $right |
||
| 308 | * @return boolean |
||
| 309 | */ |
||
| 310 | protected function isFixedModified(absences_Right $right = null) |
||
| 331 | |||
| 332 | |||
| 333 | |||
| 334 | /** |
||
| 335 | * Update rights beneficiaries |
||
| 336 | * @param absences_Right $right |
||
| 337 | * @param Widget_ProgressBar $progress |
||
| 338 | * @param array $checked_collections |
||
| 339 | */ |
||
| 340 | public static function updateCollectionsBeneficiaries(absences_Right $right, Widget_ProgressBar $progress, Array $checked_collections) |
||
| 413 | |||
| 414 | |||
| 415 | /** |
||
| 416 | * Update fixed vacation periods on an existing right |
||
| 417 | * |
||
| 418 | */ |
||
| 419 | public static function updateFixed(absences_Right $right, Widget_ProgressBar $progress) |
||
| 526 | |||
| 527 | |||
| 528 | |||
| 529 | /** |
||
| 530 | * Save posted values |
||
| 531 | * @return boolean |
||
| 532 | */ |
||
| 533 | public static function save() |
||
| 534 | { |
||
| 535 | global $babBody, $babDB; |
||
| 536 | |||
| 537 | |||
| 538 | self::$post = $_POST['right']; |
||
| 539 | |||
| 540 | |||
| 541 | if (!self::checkPost()) |
||
| 542 | { |
||
| 543 | return false; |
||
| 544 | } |
||
| 545 | |||
| 546 | |||
| 547 | if (absences_Right::CET === ((int) self::$post['kind']) || absences_Right::FIXED === ((int) self::$post['kind'])) |
||
| 548 | { |
||
| 549 | $use_in_cet = 0; |
||
| 550 | $cet_quantity = 0.0; |
||
| 551 | } else { |
||
| 552 | $use_in_cet = self::$post['use_in_cet']; |
||
| 553 | $cet_quantity = str_replace(',', '.', self::$post['cet_quantity']); |
||
| 554 | } |
||
| 555 | |||
| 556 | |||
| 557 | |||
| 558 | |||
| 559 | if (isset(self::$post['report']) && self::$post['report']) |
||
| 560 | { |
||
| 561 | $id_report_type = self::$post['id_report_type']; |
||
| 562 | $date_end_report = self::getDateValue('date_end_report'); |
||
| 563 | $description_report = self::$post['description_report']; |
||
| 564 | } else { |
||
| 565 | $id_report_type = 0; |
||
| 566 | $date_end_report = '0000-00-00'; |
||
| 567 | $description_report = ''; |
||
| 568 | } |
||
| 569 | |||
| 570 | |||
| 571 | |||
| 572 | if (!empty(self::$post['id'])) |
||
| 573 | { |
||
| 574 | |||
| 575 | $right = new absences_Right(self::$post['id']); |
||
| 576 | |||
| 577 | if (!$right->getRow()) |
||
| 578 | { |
||
| 579 | // on ne doit pas passer ici, normallement |
||
| 580 | $babBody->addError(absences_translate("This right does not exists")); |
||
| 581 | return false; |
||
| 582 | } |
||
| 583 | |||
| 584 | View Code Duplication | if($right->date_begin_fixed != '0000-00-00 00:00:00' && absences_Right::FIXED !== (int) self::$post['kind']) |
|
| 585 | { |
||
| 586 | $babBody->addError(absences_translate("A fixed vacation right cannot be changed to another right because vacation entries can be associated with it, please delete the right to remove all vacation entries created by this right")); |
||
| 587 | return false; |
||
| 588 | } |
||
| 589 | |||
| 590 | |||
| 591 | $fixedModified = self::isFixedModified($right); |
||
| 592 | |||
| 593 | |||
| 594 | $babDB->db_query(" |
||
| 595 | |||
| 596 | UPDATE ".ABSENCES_RIGHTS_TBL." |
||
| 597 | SET |
||
| 598 | kind =".$babDB->quote(self::$post['kind']).", |
||
| 599 | description =".$babDB->quote(self::$post['description']).", |
||
| 600 | id_creditor =".$babDB->quote($GLOBALS['BAB_SESS_USERID']).", |
||
| 601 | id_type =".$babDB->quote(self::$post['id_type']).", |
||
| 602 | quantity =".$babDB->quote(self::$post['quantity']).", |
||
| 603 | quantity_unit =".$babDB->quote(self::$post['quantity_unit']).", |
||
| 604 | date_entry =curdate(), |
||
| 605 | date_begin =".$babDB->quote(self::getDateValue('date_begin')).", |
||
| 606 | date_end =".$babDB->quote(self::getDateValue('date_end')).", |
||
| 607 | active =".$babDB->quote(self::$post['active']).", |
||
| 608 | cbalance =".$babDB->quote(self::$post['cbalance']).", |
||
| 609 | date_begin_valid =".$babDB->quote(self::getDateValue('date_begin_valid')).", |
||
| 610 | date_end_valid =".$babDB->quote(self::getDateValue('date_end_valid')).", |
||
| 611 | date_begin_fixed =".$babDB->quote(self::getDateTimeValue('datebeginfx', 'hourbeginfx')).", |
||
| 612 | date_end_fixed =".$babDB->quote(self::getDateTimeValue('dateendfx', 'hourendfx')).", |
||
| 613 | no_distribution =".$babDB->quote(self::$post['no_distribution']).", |
||
| 614 | use_in_cet =".$babDB->quote($use_in_cet).", |
||
| 615 | cet_quantity =".$babDB->quote($cet_quantity).", |
||
| 616 | id_rgroup =".$babDB->quote(self::$post['id_rgroup']).", |
||
| 617 | earlier =".$babDB->quote(self::$post['earlier']).", |
||
| 618 | earlier_begin_valid =".$babDB->quote(self::$post['earlier_begin_valid']).", |
||
| 619 | earlier_end_valid =".$babDB->quote(self::$post['earlier_end_valid']).", |
||
| 620 | later =".$babDB->quote(self::$post['later']).", |
||
| 621 | later_begin_valid =".$babDB->quote(self::$post['later_begin_valid']).", |
||
| 622 | later_end_valid =".$babDB->quote(self::$post['later_end_valid']).", |
||
| 623 | delay_before =".$babDB->quote(self::$post['delay_before']).", |
||
| 624 | id_report_type =".$babDB->quote($id_report_type).", |
||
| 625 | date_end_report =".$babDB->quote($date_end_report).", |
||
| 626 | description_report =".$babDB->quote($description_report).", |
||
| 627 | quantity_alert_days =".$babDB->quote(self::$post['quantity_alert_days']).", |
||
| 628 | quantity_alert_types =".$babDB->quote(isset(self::$post['quantity_alert_types']) ? implode(',',self::$post['quantity_alert_types']) : '').", |
||
| 629 | quantity_alert_begin =".$babDB->quote(self::getDateValue('quantity_alert_begin')).", |
||
| 630 | quantity_alert_end =".$babDB->quote(self::getDateValue('quantity_alert_end')).", |
||
| 631 | quantity_inc_month =".$babDB->quote(str_replace(',', '.', self::$post['quantity_inc_month'])).", |
||
| 632 | quantity_inc_max =".$babDB->quote(str_replace(',', '.', self::$post['quantity_inc_max'])).", |
||
| 633 | |||
| 634 | dynconf_types =".$babDB->quote(isset(self::$post['dynconf_types']) ? implode(',',self::$post['dynconf_types']) : '').", |
||
| 635 | dynconf_begin =".$babDB->quote(self::getDateValue('dynconf_begin')).", |
||
| 636 | dynconf_end =".$babDB->quote(self::getDateValue('dynconf_end')).", |
||
| 637 | |||
| 638 | require_approval =".$babDB->quote(self::$post['require_approval'])." |
||
| 639 | WHERE |
||
| 640 | id=".$babDB->quote(self::$post['id'])." |
||
| 641 | |||
| 642 | "); |
||
| 643 | |||
| 644 | $id = self::$post['id']; |
||
| 645 | |||
| 646 | $right->addMovement(sprintf(absences_translate('Vacation right updated : %s'), self::$post['description'])); |
||
| 647 | |||
| 648 | // si la quantite du droit a ete changee, enregistrer un mouvement pour tout les utilisateurs concernes |
||
| 649 | if (!absences_cq($right->quantity, self::$post['quantity']) || $right->quantity_unit !== self::$post['quantity_unit']) |
||
| 650 | { |
||
| 651 | foreach($right->getAgentRightIterator() as $agentRight) |
||
| 652 | { |
||
| 653 | /*@var $agentRight absences_AgentRight */ |
||
| 654 | if ($agentRight->quantity == '') |
||
| 655 | { |
||
| 656 | $agentRight->addMovement(sprintf(absences_translate('The quantity on right %s has been modified from %s to %s for the user %s'), |
||
| 657 | self::$post['description'], |
||
| 658 | absences_quantity($right->quantity, $right->quantity_unit), |
||
| 659 | absences_quantity(self::$post['quantity'], self::$post['quantity_unit']), |
||
| 660 | $agentRight->getAgent()->getName() |
||
| 661 | )); |
||
| 662 | } |
||
| 663 | } |
||
| 664 | } |
||
| 665 | } |
||
| 666 | else |
||
| 667 | { |
||
| 668 | |||
| 669 | require_once $GLOBALS['babInstallPath'].'utilit/uuid.php'; |
||
| 670 | |||
| 671 | $fixedModified = self::isFixedModified(null); |
||
| 672 | |||
| 673 | if (!isset(self::$post['active'])) { |
||
| 674 | self::$post['active'] = 'N'; |
||
| 675 | } |
||
| 676 | |||
| 677 | |||
| 678 | $babDB->db_query(" |
||
| 679 | |||
| 680 | INSERT into ".ABSENCES_RIGHTS_TBL." |
||
| 681 | ( |
||
| 682 | kind, |
||
| 683 | description, |
||
| 684 | id_creditor, |
||
| 685 | id_type, |
||
| 686 | quantity, |
||
| 687 | quantity_unit, |
||
| 688 | createdOn, |
||
| 689 | date_entry, |
||
| 690 | date_begin, |
||
| 691 | date_end, |
||
| 692 | active, |
||
| 693 | cbalance, |
||
| 694 | date_begin_valid, |
||
| 695 | date_end_valid, |
||
| 696 | date_begin_fixed, |
||
| 697 | date_end_fixed, |
||
| 698 | no_distribution, |
||
| 699 | use_in_cet, |
||
| 700 | cet_quantity, |
||
| 701 | id_rgroup, |
||
| 702 | |||
| 703 | earlier, |
||
| 704 | earlier_begin_valid, |
||
| 705 | earlier_end_valid, |
||
| 706 | later, |
||
| 707 | later_begin_valid, |
||
| 708 | later_end_valid, |
||
| 709 | delay_before, |
||
| 710 | |||
| 711 | id_report_type, |
||
| 712 | date_end_report, |
||
| 713 | description_report, |
||
| 714 | quantity_alert_days, |
||
| 715 | quantity_alert_types, |
||
| 716 | quantity_alert_begin, |
||
| 717 | quantity_alert_end, |
||
| 718 | quantity_inc_month, |
||
| 719 | quantity_inc_max, |
||
| 720 | |||
| 721 | dynconf_types, |
||
| 722 | dynconf_begin, |
||
| 723 | dynconf_end, |
||
| 724 | |||
| 725 | require_approval, |
||
| 726 | uuid |
||
| 727 | ) |
||
| 728 | VALUES |
||
| 729 | ( |
||
| 730 | ".$babDB->quote(self::$post['kind']).", |
||
| 731 | ".$babDB->quote(self::$post['description']).", |
||
| 732 | ".$babDB->quote($GLOBALS['BAB_SESS_USERID']).", |
||
| 733 | ".$babDB->quote(self::$post['id_type']).", |
||
| 734 | ".$babDB->quote(self::$post['quantity']).", |
||
| 735 | ".$babDB->quote(self::$post['quantity_unit']).", |
||
| 736 | curdate(), |
||
| 737 | curdate(), |
||
| 738 | ".$babDB->quote(self::getDateValue('date_begin')).", |
||
| 739 | ".$babDB->quote(self::getDateValue('date_end')).", |
||
| 740 | ".$babDB->quote(self::$post['active']).", |
||
| 741 | ".$babDB->quote(self::$post['cbalance']).", |
||
| 742 | ".$babDB->quote(self::getDateValue('date_begin_valid')).", |
||
| 743 | ".$babDB->quote(self::getDateValue('date_end_valid')).", |
||
| 744 | ".$babDB->quote(self::getDateTimeValue('datebeginfx', 'hourbeginfx')).", |
||
| 745 | ".$babDB->quote(self::getDateTimeValue('dateendfx', 'hourendfx')).", |
||
| 746 | ".$babDB->quote(self::$post['no_distribution']).", |
||
| 747 | ".$babDB->quote($use_in_cet).", |
||
| 748 | ".$babDB->quote($cet_quantity).", |
||
| 749 | ".$babDB->quote(self::$post['id_rgroup']).", |
||
| 750 | |||
| 751 | ".$babDB->quote(self::$post['earlier']).", |
||
| 752 | ".$babDB->quote(self::$post['earlier_begin_valid']).", |
||
| 753 | ".$babDB->quote(self::$post['earlier_end_valid']).", |
||
| 754 | ".$babDB->quote(self::$post['later']).", |
||
| 755 | ".$babDB->quote(self::$post['later_begin_valid']).", |
||
| 756 | ".$babDB->quote(self::$post['later_end_valid']).", |
||
| 757 | ".$babDB->quote(self::$post['delay_before']).", |
||
| 758 | |||
| 759 | ".$babDB->quote($id_report_type).", |
||
| 760 | ".$babDB->quote($date_end_report).", |
||
| 761 | ".$babDB->quote($description_report).", |
||
| 762 | ".$babDB->quote(self::$post['quantity_alert_days']).", |
||
| 763 | ".$babDB->quote(isset(self::$post['quantity_alert_types']) ? implode(',',self::$post['quantity_alert_types']) : '').", |
||
| 764 | ".$babDB->quote(self::getDateValue('quantity_alert_begin')).", |
||
| 765 | ".$babDB->quote(self::getDateValue('quantity_alert_end')).", |
||
| 766 | ".$babDB->quote(str_replace(',', '.', self::$post['quantity_inc_month'])).", |
||
| 767 | ".$babDB->quote(str_replace(',', '.', self::$post['quantity_inc_max'])).", |
||
| 768 | |||
| 769 | ".$babDB->quote(isset(self::$post['dynconf_types']) ? implode(',',self::$post['dynconf_types']) : '').", |
||
| 770 | ".$babDB->quote(self::getDateValue('dynconf_begin')).", |
||
| 771 | ".$babDB->quote(self::getDateValue('dynconf_end')).", |
||
| 772 | |||
| 773 | ".$babDB->quote(self::$post['require_approval']).", |
||
| 774 | ".$babDB->quote(bab_uuid())." |
||
| 775 | ) |
||
| 776 | "); |
||
| 777 | |||
| 778 | $id = $babDB->db_insert_id(); |
||
| 779 | |||
| 780 | |||
| 781 | $movement = new absences_Movement; |
||
| 782 | $movement->id_right = $id; |
||
| 783 | $movement->message = sprintf(absences_translate('Vacation right created : %s'), self::$post['description']); |
||
| 784 | $movement->save(); |
||
| 785 | } |
||
| 786 | |||
| 787 | self::saveRules($id); |
||
| 788 | self::saveCet($id); |
||
| 789 | self::applyDynamicRights($id); |
||
| 790 | |||
| 791 | |||
| 792 | // si aucun beneficiaire, proposer d'en ajouter |
||
| 793 | $res = $babDB->db_query('SELECT id FROM absences_users_rights WHERE id_right='.$babDB->quote($id)); |
||
| 794 | if ($babDB->db_num_rows($res) === 0) |
||
| 795 | { |
||
| 796 | absences_RightAct::goToNoBeneficiaries($id); |
||
| 797 | } |
||
| 798 | |||
| 799 | |||
| 800 | // if fixed, update fixed requests |
||
| 801 | if ($fixedModified) |
||
| 802 | { |
||
| 803 | absences_RightAct::goToUpdateFixed($id); |
||
| 804 | } |
||
| 805 | |||
| 806 | |||
| 807 | // to list |
||
| 808 | absences_RightAct::redirect(); |
||
| 809 | } |
||
| 810 | |||
| 811 | |||
| 812 | |||
| 813 | /** |
||
| 814 | * Apply dynamic rights for current beneficiaries |
||
| 815 | * @param int $id |
||
| 816 | */ |
||
| 817 | protected static function applyDynamicRights($id) |
||
| 827 | |||
| 828 | |||
| 829 | |||
| 830 | protected static function saveCet($id) |
||
| 907 | |||
| 908 | |||
| 909 | |||
| 910 | /** |
||
| 911 | * Save rules associated to right |
||
| 912 | * @param int $id |
||
| 913 | */ |
||
| 914 | protected static function saveRules($id) |
||
| 1054 | |||
| 1055 | |||
| 1056 | View Code Duplication | public static function goToNoBeneficiaries($id) |
|
| 1065 | |||
| 1066 | |||
| 1067 | View Code Duplication | public static function goToUpdateFixed($id) |
|
| 1076 | |||
| 1077 | |||
| 1078 | View Code Duplication | public static function redirect() |
|
| 1086 | |||
| 1087 | } |
||
| 1088 |
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.