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 PermissionAdmin 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 PermissionAdmin, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
28 | class PermissionAdmin |
||
29 | { |
||
30 | const ADD = 'ADD'; |
||
31 | const DELETE = 'DEL'; |
||
32 | |||
33 | /** |
||
34 | * @var AbstractEntity |
||
35 | */ |
||
36 | protected $resource = null; |
||
37 | |||
38 | /** |
||
39 | * @var EntityManager |
||
40 | */ |
||
41 | protected $em = null; |
||
42 | |||
43 | /** |
||
44 | * @var TokenStorageInterface |
||
45 | */ |
||
46 | protected $tokenStorage = null; |
||
47 | |||
48 | /** |
||
49 | * @var MutableAclProviderInterface |
||
50 | */ |
||
51 | protected $aclProvider = null; |
||
52 | |||
53 | /** |
||
54 | * @var ObjectIdentityRetrievalStrategyInterface |
||
55 | */ |
||
56 | protected $oidRetrievalStrategy = null; |
||
57 | |||
58 | /** |
||
59 | * @var PermissionMap |
||
60 | */ |
||
61 | protected $permissionMap = null; |
||
62 | |||
63 | /** |
||
64 | * @var array |
||
65 | */ |
||
66 | protected $permissions = null; |
||
67 | |||
68 | /** |
||
69 | * @var EventDispatcherInterface |
||
70 | */ |
||
71 | protected $eventDispatcher = null; |
||
72 | |||
73 | /** |
||
74 | * @var KernelInterface |
||
75 | */ |
||
76 | protected $kernel; |
||
77 | |||
78 | /** |
||
79 | * @var Shell |
||
80 | */ |
||
81 | protected $shellHelper; |
||
82 | |||
83 | /** |
||
84 | * Constructor |
||
85 | * |
||
86 | * @param EntityManager $em The EntityManager |
||
87 | * @param TokenStorageInterface $tokenStorage The token storage |
||
88 | * @param AclProviderInterface $aclProvider The ACL provider |
||
89 | * @param ObjectIdentityRetrievalStrategyInterface $oidRetrievalStrategy The object retrieval strategy |
||
90 | * @param EventDispatcherInterface $eventDispatcher The event dispatcher |
||
91 | * @param Shell $shellHelper The shell helper |
||
92 | * @param KernelInterface $kernel The kernel |
||
93 | */ |
||
94 | 14 | public function __construct( |
|
111 | |||
112 | /** |
||
113 | * Initialize permission admin with specified entity. |
||
114 | * |
||
115 | * @param AbstractEntity $resource The object which has the permissions |
||
116 | * @param PermissionMapInterface $permissionMap The permission map to use |
||
117 | */ |
||
118 | 12 | public function initialize(AbstractEntity $resource, PermissionMapInterface $permissionMap) |
|
141 | |||
142 | /** |
||
143 | * Get permissions. |
||
144 | * |
||
145 | * @return MaskBuilder[] |
||
146 | */ |
||
147 | 1 | public function getPermissions() |
|
151 | |||
152 | /** |
||
153 | * Get permission for specified role. |
||
154 | * |
||
155 | * @param RoleInterface|string $role |
||
156 | * |
||
157 | * @return MaskBuilder|null |
||
158 | */ |
||
159 | 3 | public function getPermission($role) |
|
170 | |||
171 | /** |
||
172 | * Get all roles. |
||
173 | * |
||
174 | * @return Role[] |
||
175 | */ |
||
176 | 1 | public function getAllRoles() |
|
180 | |||
181 | /** |
||
182 | * Get all manageable roles for pages |
||
183 | * |
||
184 | * @return Role[] |
||
185 | */ |
||
186 | 1 | public function getManageableRolesForPages() |
|
199 | |||
200 | /** |
||
201 | * Get possible permissions. |
||
202 | * |
||
203 | * @return array |
||
204 | */ |
||
205 | 1 | public function getPossiblePermissions() |
|
209 | |||
210 | /** |
||
211 | * Handle form entry of permission changes. |
||
212 | * |
||
213 | * @param Request $request |
||
214 | * |
||
215 | * @return bool |
||
216 | */ |
||
217 | 2 | public function bindRequest(Request $request) |
|
243 | |||
244 | /** |
||
245 | * Create a new ACL changeset. |
||
246 | * |
||
247 | * @param AbstractEntity $entity The entity |
||
248 | * @param array $changes The changes |
||
249 | * @param UserInterface $user The user |
||
250 | * |
||
251 | * @return AclChangeset |
||
252 | */ |
||
253 | 2 | public function createAclChangeSet(AbstractEntity $entity, $changes, UserInterface $user) |
|
265 | |||
266 | /** |
||
267 | * Apply the specified ACL changeset. |
||
268 | * |
||
269 | * @param AbstractEntity $entity The entity |
||
270 | * @param array $changeset The changeset |
||
271 | * @param bool $recursive The recursive |
||
272 | */ |
||
273 | 4 | public function applyAclChangeset(AbstractEntity $entity, $changeset, $recursive = true) |
|
329 | |||
330 | /** |
||
331 | * Get current object ACE index for specified role. |
||
332 | * |
||
333 | * @param AclInterface $acl The AclInterface |
||
334 | * @param string $role The role |
||
335 | * |
||
336 | * @return bool|int |
||
337 | */ |
||
338 | 2 | View Code Duplication | private function getObjectAceIndex(AclInterface $acl, $role) |
351 | |||
352 | /** |
||
353 | * Get object ACE mask at specified index. |
||
354 | * |
||
355 | * @param AclInterface $acl The acl interface |
||
356 | * @param int $index The index |
||
357 | * |
||
358 | * @return bool|int |
||
359 | */ |
||
360 | 2 | View Code Duplication | private function getMaskAtIndex(AclInterface $acl, $index) |
372 | } |
||
373 |
The
EntityManager
might become unusable for example if a transaction is rolled back and it gets closed. Let’s assume that somewhere in your application, or in a third-party library, there is code such as the following:If that code throws an exception and the
EntityManager
is closed. Any other code which depends on the same instance of theEntityManager
during this request will fail.On the other hand, if you instead inject the
ManagerRegistry
, thegetManager()
method guarantees that you will always get a usable manager instance.