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 PMF_Perm_Medium 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 PMF_Perm_Medium, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
35 | class PMF_Perm_Medium extends PMF_Perm_Basic |
||
36 | { |
||
37 | /** |
||
38 | * Default data for new groups. |
||
39 | * |
||
40 | * @var array |
||
41 | */ |
||
42 | public $defaultGroupData = array( |
||
43 | 'name' => 'DEFAULT_GROUP', |
||
44 | 'description' => 'Short group description.', |
||
45 | 'auto_join' => false |
||
46 | ); |
||
47 | |||
48 | /** |
||
49 | * Constructor |
||
50 | * |
||
51 | * @param PMF_Configuration $config |
||
52 | * |
||
53 | * @return PMF_Perm_Medium |
||
54 | */ |
||
55 | public function __construct(PMF_Configuration $config) |
||
59 | |||
60 | /** |
||
61 | * Returns true if the group specified by $groupId owns the |
||
62 | * right given by $rightId, otherwise false. |
||
63 | * |
||
64 | * @param integer $groupId Group ID |
||
65 | * @param integer $rightId Right ID |
||
66 | * |
||
67 | * @return bool |
||
68 | */ |
||
69 | View Code Duplication | public function checkGroupRight($groupId, $rightId) |
|
102 | |||
103 | /** |
||
104 | * Returns an array that contains the right-IDs of all |
||
105 | * group-rights the group $groupId owns. |
||
106 | * |
||
107 | * @param integer $groupId Group ID |
||
108 | * |
||
109 | * @return array |
||
110 | */ |
||
111 | View Code Duplication | public function getGroupRights($groupId) |
|
|
|||
112 | { |
||
113 | if ($groupId <= 0 || !is_numeric($groupId)) { |
||
114 | return false; |
||
115 | } |
||
116 | // check right |
||
117 | $select = sprintf(" |
||
118 | SELECT |
||
119 | fr.right_id AS right_id |
||
120 | FROM |
||
121 | %sfaqright fr, |
||
122 | %sfaqgroup_right fgr, |
||
123 | %sfaqgroup fg |
||
124 | WHERE |
||
125 | fg.group_id = %d AND |
||
126 | fg.group_id = fgr.group_id AND |
||
127 | fr.right_id = fgr.right_id", |
||
128 | PMF_Db::getTablePrefix(), |
||
129 | PMF_Db::getTablePrefix(), |
||
130 | PMF_Db::getTablePrefix(), |
||
131 | $groupId |
||
132 | ); |
||
133 | |||
134 | $res = $this->config->getDb()->query($select); |
||
135 | $result = []; |
||
136 | while ($row = $this->config->getDb()->fetchArray($res)) { |
||
137 | $result[] = $row['right_id']; |
||
138 | } |
||
139 | return $result; |
||
140 | } |
||
141 | |||
142 | /** |
||
143 | * Returns true, if the user given by $userId owns the right |
||
144 | * specified by $right. It does not matter if the user owns this |
||
145 | * right as a user-right or because of a group-membership. |
||
146 | * The parameter $right may be a right-ID (recommended for |
||
147 | * performance) or a right-name. |
||
148 | * |
||
149 | * @param integer $userId Group ID |
||
150 | * @param mixed $right Rights |
||
151 | * |
||
152 | * @return boolean |
||
153 | */ |
||
154 | public function checkRight($userId, $right) |
||
167 | |||
168 | /** |
||
169 | * Grants the group given by $groupId the right specified by |
||
170 | * $rightId. |
||
171 | * |
||
172 | * @param integer $groupId Group ID |
||
173 | * @param integer $rightId Right ID |
||
174 | * |
||
175 | * @return boolean |
||
176 | */ |
||
177 | public function grantGroupRight($groupId, $rightId) |
||
208 | |||
209 | /** |
||
210 | * Refuses the group given by $groupId the right specified by |
||
211 | * $rightId. |
||
212 | * |
||
213 | * @param integer $groupId Group ID |
||
214 | * @param integer $rightId Right ID |
||
215 | * @return boolean |
||
216 | */ |
||
217 | View Code Duplication | public function refuseGroupRight($groupId, $rightId) |
|
242 | |||
243 | /** |
||
244 | * Adds a new group to the database and returns the ID of the |
||
245 | * new group. The associative array $groupData contains the |
||
246 | * data for the new group. |
||
247 | * |
||
248 | * @param array $groupData Array of group data |
||
249 | * |
||
250 | * @return int |
||
251 | */ |
||
252 | public function addGroup(Array $groupData) |
||
280 | |||
281 | /** |
||
282 | * Changes the group data of the given group. |
||
283 | * |
||
284 | * @param integer $groupId Group ID |
||
285 | * @param array $groupData Array of group data |
||
286 | * |
||
287 | * @return boolean |
||
288 | */ |
||
289 | View Code Duplication | public function changeGroup($groupId, Array $groupData) |
|
319 | |||
320 | /** |
||
321 | * Removes the group given by $groupId from the database. |
||
322 | * Returns true on success, otherwise false. |
||
323 | * |
||
324 | * @param integer $groupId Group ID |
||
325 | * |
||
326 | * @return boolean |
||
327 | */ |
||
328 | public function deleteGroup($groupId) |
||
378 | |||
379 | /** |
||
380 | * Returns true if the user given by $userId is a member of |
||
381 | * the group specified by $groupId, otherwise false. |
||
382 | * |
||
383 | * @param integer $userId User ID |
||
384 | * @param integer $groupId Group ID |
||
385 | * |
||
386 | * @return boolean |
||
387 | */ |
||
388 | View Code Duplication | public function isGroupMember($userId, $groupId) |
|
419 | |||
420 | /** |
||
421 | * Returns an array that contains the user-IDs of all members |
||
422 | * of the group $groupId. |
||
423 | * |
||
424 | * @param integer $groupId Group ID |
||
425 | * |
||
426 | * @return array |
||
427 | */ |
||
428 | View Code Duplication | public function getGroupMembers($groupId) |
|
429 | { |
||
430 | if ($groupId <= 0 || !is_numeric($groupId)) { |
||
431 | return false; |
||
432 | } |
||
433 | |||
434 | $select = sprintf(" |
||
435 | SELECT |
||
436 | fu.user_id AS user_id |
||
437 | FROM |
||
438 | %sfaquser fu, |
||
439 | %sfaquser_group fug, |
||
440 | %sfaqgroup fg |
||
441 | WHERE |
||
442 | fg.group_id = %d AND |
||
443 | fg.group_id = fug.group_id AND |
||
444 | fu.user_id = fug.user_id", |
||
445 | PMF_Db::getTablePrefix(), |
||
446 | PMF_Db::getTablePrefix(), |
||
447 | PMF_Db::getTablePrefix(), |
||
448 | $groupId |
||
449 | ); |
||
450 | |||
451 | $res = $this->config->getDb()->query($select); |
||
452 | $result = []; |
||
453 | while ($row = $this->config->getDb()->fetchArray($res)) { |
||
454 | $result[] = $row['user_id']; |
||
455 | } |
||
456 | return $result; |
||
457 | } |
||
458 | |||
459 | /** |
||
460 | * Adds a new member $userId to the group $groupId. |
||
461 | * Returns true on success, otherwise false. |
||
462 | * |
||
463 | * @param integer $userId User ID |
||
464 | * @param integer $groupId Group ID |
||
465 | * |
||
466 | * @return boolean |
||
467 | */ |
||
468 | View Code Duplication | function addToGroup($userId, $groupId) |
|
496 | |||
497 | /** |
||
498 | * Removes a user $userId from the group $groupId. |
||
499 | * Returns true on success, otherwise false. |
||
500 | * |
||
501 | * @param integer $userId User ID |
||
502 | * @param integer $groupId Group ID |
||
503 | * |
||
504 | * @return boolean |
||
505 | */ |
||
506 | View Code Duplication | public function removeFromGroup($userId, $groupId) |
|
528 | |||
529 | /** |
||
530 | * Returns the ID of the group that has the name $name. Returns |
||
531 | * 0 if the group-name cannot be found. |
||
532 | * |
||
533 | * @param string $name Group name |
||
534 | * |
||
535 | * @return int |
||
536 | */ |
||
537 | View Code Duplication | public function getGroupId($name) |
|
557 | |||
558 | /** |
||
559 | * Returns an associative array with the group-data of the group |
||
560 | * $groupId. |
||
561 | * |
||
562 | * @param integer $groupId Group ID |
||
563 | * |
||
564 | * @return array |
||
565 | */ |
||
566 | View Code Duplication | public function getGroupData($groupId) |
|
592 | |||
593 | /** |
||
594 | * Returns an array that contains the IDs of all groups in which |
||
595 | * the user $userId is a member. |
||
596 | * |
||
597 | * @param integer $userId User ID |
||
598 | * |
||
599 | * @return array |
||
600 | */ |
||
601 | public function getUserGroups($userId) |
||
631 | |||
632 | /** |
||
633 | * Returns an array with the IDs of all groups stored in the |
||
634 | * database. |
||
635 | * |
||
636 | * @return array |
||
637 | */ |
||
638 | View Code Duplication | public function getAllGroups() |
|
656 | |||
657 | /** |
||
658 | * Get all groups in <option> tags |
||
659 | * |
||
660 | * @param array $groups Selected groups |
||
661 | * |
||
662 | * @return string |
||
663 | */ |
||
664 | public function getAllGroupsOptions(Array $groups) |
||
681 | |||
682 | /** |
||
683 | * checkUserGroupRight |
||
684 | * |
||
685 | * Returns true if the user $userId owns the right $rightId |
||
686 | * because of a group-membership, otherwise false. |
||
687 | * |
||
688 | * @param integer $userId User ID |
||
689 | * @param integer $rightId Right ID |
||
690 | * @return boolean |
||
691 | */ |
||
692 | View Code Duplication | public function checkUserGroupRight($userId, $rightId) |
|
730 | |||
731 | /** |
||
732 | * Checks the given associative array $groupData. If a |
||
733 | * parameter is incorrect or is missing, it will be replaced |
||
734 | * by the default values in $this->defaultGroupData. |
||
735 | * Returns the corrected $groupData associative array. |
||
736 | * |
||
737 | * @param array $groupData Array of group data |
||
738 | * @return array |
||
739 | */ |
||
740 | public function checkGroupData(Array $groupData) |
||
755 | |||
756 | /** |
||
757 | * Returns an array that contains the right-IDs of all rights |
||
758 | * the user $userId owns. User-rights and the rights the user |
||
759 | * owns because of a group-membership are taken into account. |
||
760 | * |
||
761 | * @param integer $userId User ID |
||
762 | * @return array |
||
763 | */ |
||
764 | public function getAllUserRights($userId) |
||
774 | |||
775 | /** |
||
776 | * Adds the user $userId to all groups with the auto_join |
||
777 | * option. By using the auto_join option, user administration |
||
778 | * can be much easier. For example by setting this option only |
||
779 | * for a single group called 'All Users'. The autoJoin() method |
||
780 | * then has to be called every time a new user registers. |
||
781 | * Returns true on success, otherwise false. |
||
782 | * |
||
783 | * @param integer $userId User ID |
||
784 | * @return boolean |
||
785 | */ |
||
786 | public function autoJoin($userId) |
||
818 | |||
819 | /** |
||
820 | * Removes the user $userId from all groups. |
||
821 | * Returns true on success, otherwise false. |
||
822 | * |
||
823 | * @param integer $userId User ID |
||
824 | * @return boolean |
||
825 | */ |
||
826 | View Code Duplication | public function removeFromAllGroups($userId) |
|
846 | |||
847 | /** |
||
848 | * getUserGroupRights |
||
849 | * |
||
850 | * Returns an array that contains the IDs of all rights the user |
||
851 | * $userId owns because of a group-membership. |
||
852 | * |
||
853 | * @param integer $userId User ID |
||
854 | * @return array |
||
855 | */ |
||
856 | View Code Duplication | public function getUserGroupRights($userId) |
|
857 | { |
||
858 | if ($userId <= 0 || !is_numeric($userId)) { |
||
859 | return false; |
||
860 | } |
||
861 | |||
862 | $select = sprintf(" |
||
863 | SELECT |
||
864 | fr.right_id AS right_id |
||
865 | FROM |
||
866 | %sfaqright fr, |
||
867 | %sfaqgroup_right fgr, |
||
868 | %sfaqgroup fg, |
||
869 | %sfaquser_group fug, |
||
870 | %sfaquser fu |
||
871 | WHERE |
||
872 | fu.user_id = %d AND |
||
873 | fu.user_id = fug.user_id AND |
||
874 | fg.group_id = fug.group_id AND |
||
875 | fg.group_id = fgr.group_id AND |
||
876 | fr.right_id = fgr.right_id", |
||
877 | PMF_Db::getTablePrefix(), |
||
878 | PMF_Db::getTablePrefix(), |
||
879 | PMF_Db::getTablePrefix(), |
||
880 | PMF_Db::getTablePrefix(), |
||
881 | PMF_Db::getTablePrefix(), |
||
882 | $userId); |
||
883 | |||
884 | $res = $this->config->getDb()->query($select); |
||
885 | $result = []; |
||
886 | while ($row = $this->config->getDb()->fetchArray($res)) { |
||
887 | $result[] = $row['right_id']; |
||
888 | } |
||
889 | return $result; |
||
890 | } |
||
891 | |||
892 | /** |
||
893 | * Refuses all group rights. |
||
894 | * Returns true on success, otherwise false. |
||
895 | * |
||
896 | * @param integer $groupId Group ID |
||
897 | * @return boolean |
||
898 | */ |
||
899 | View Code Duplication | public function refuseAllGroupRights($groupId) |
|
919 | |||
920 | /** |
||
921 | * Returns the name of the group $groupId. |
||
922 | * |
||
923 | * @param integer $groupId Group ID |
||
924 | * @return string |
||
925 | */ |
||
926 | View Code Duplication | public function getGroupName($groupId) |
|
950 | |||
951 | /** |
||
952 | * Removes all users from the group $groupId. |
||
953 | * Returns true on success, otherwise false. |
||
954 | * |
||
955 | * @param integer $groupId Group ID |
||
956 | * @return bool |
||
957 | */ |
||
958 | View Code Duplication | public function removeAllUsersFromGroup($groupId) |
|
979 | } |
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.