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 UserAndLevel 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 UserAndLevel, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
12 | trait UserAndLevel |
||
13 | { |
||
14 | /** |
||
15 | * USER & LEVEL |
||
16 | * Check if a model has a given level |
||
17 | * |
||
18 | * @param mixed level |
||
19 | 1 | * @return bool |
|
20 | */ |
||
21 | 1 | public function isLevel($level) |
|
22 | { |
||
23 | 1 | $level = $this->getALevel($level); |
|
|
|||
24 | |||
25 | return is_a($this, config('laravel-acl.level'), true) ? $this->name == $level->name : $this->level->name == $level->name; |
||
26 | } |
||
27 | |||
28 | /** |
||
29 | * USER & LEVEL |
||
30 | * Check if a user has or has inherited |
||
31 | * a given level |
||
32 | * |
||
33 | * @param $level |
||
34 | 1 | * @return bool |
|
35 | */ |
||
36 | 1 | public function hasLevel($level) |
|
37 | { |
||
38 | 1 | $level = $this->getLevel($level); |
|
39 | |||
40 | 1 | return is_a($this, config('laravel-acl.level'), true) ? $this->rank <= $level->rank : $this->level->rank <= $level->rank; |
|
41 | } |
||
42 | |||
43 | /** |
||
44 | * USER & LEVEL |
||
45 | * Add a single permission to a model |
||
46 | * |
||
47 | * @param $permission |
||
48 | */ |
||
49 | 15 | public function addPermission($permission) |
|
53 | |||
54 | /** |
||
55 | * USER & LEVEL |
||
56 | * Add an array of permissions to a model |
||
57 | * |
||
58 | * @param $permissions |
||
59 | */ |
||
60 | 9 | public function addPermissions(array $permissions) |
|
64 | |||
65 | /** |
||
66 | * USER & LEVEL |
||
67 | * Remove a permission from a model |
||
68 | * |
||
69 | * @param $permission |
||
70 | */ |
||
71 | 2 | public function removePermission($permission) |
|
75 | |||
76 | /** |
||
77 | * USER & LEVEL |
||
78 | * Remove an array of permissions objects or permission names (can be mixed) |
||
79 | * |
||
80 | * @param $permissions |
||
81 | */ |
||
82 | 1 | public function removePermissions(array $permissions) |
|
86 | |||
87 | /** |
||
88 | * USER & LEVEL |
||
89 | * Sync permissions on a model |
||
90 | * |
||
91 | * @param $permissions |
||
92 | */ |
||
93 | 2 | public function syncPermissions(array $permissions) |
|
97 | |||
98 | /** |
||
99 | * USER & LEVEL |
||
100 | * Clear permissions from a model |
||
101 | */ |
||
102 | public function clearPermissions() |
||
110 | 2 | ||
111 | 3 | /** |
|
112 | * USER & LEVEL |
||
113 | * First try the cache to return the collection, |
||
114 | * then fetch it from the database. |
||
115 | * |
||
116 | * See @cacheGetPermissions() |
||
117 | * |
||
118 | * @return \Illuminate\Support\Collection |
||
119 | */ |
||
120 | View Code Duplication | public function getPermissions() |
|
130 | |||
131 | 6 | /** |
|
132 | 6 | * USER & LEVEL |
|
133 | 6 | * First try the cache to return the collection, |
|
134 | * then fetch it from the database. |
||
135 | 6 | * |
|
136 | 6 | * See @cacheGetAllPermissions() |
|
137 | * |
||
138 | * @return \Illuminate\Support\Collection |
||
139 | */ |
||
140 | View Code Duplication | public function getAllPermissions() |
|
150 | |||
151 | 3 | /** |
|
152 | * USER & LEVEL |
||
153 | 3 | * First try the cache to return the collection, |
|
154 | 3 | * then fetch it from the database. |
|
155 | 3 | * |
|
156 | * See @cacheGetInheritedPermissions() |
||
157 | 3 | * |
|
158 | 3 | * @return \Illuminate\Support\Collection |
|
159 | */ |
||
160 | View Code Duplication | public function getInheritedPermissions() |
|
170 | |||
171 | 2 | /** |
|
172 | * USER & LEVEL |
||
173 | 2 | * First try the cache to return the collection, |
|
174 | * then fetch it from the database. |
||
175 | 2 | * |
|
176 | 2 | * See @cacheGetAvailablePermissions() |
|
177 | 2 | * |
|
178 | * @return \Illuminate\Support\Collection |
||
179 | 2 | */ |
|
180 | 2 | View Code Duplication | public function getAvailablePermissions() |
190 | |||
191 | /** |
||
192 | * USER & LEVEL |
||
193 | * First try the cache to return the collection, |
||
194 | 2 | * then fetch it from the database. |
|
195 | * |
||
196 | 2 | * See @cacheHasPermissionTo() |
|
197 | * |
||
198 | 2 | * @param $permission |
|
199 | 2 | * @return bool |
|
200 | 2 | */ |
|
201 | public function hasPermissionTo($permission) |
||
211 | |||
212 | /** |
||
213 | * USER & LEVEL |
||
214 | * Returns if a level is lower ranking than another level, |
||
215 | 3 | * or if a user is lower ranking than another user. |
|
216 | * |
||
217 | 3 | * @param $other |
|
218 | 3 | * @return bool |
|
219 | 3 | */ |
|
220 | public function isLowerThan($other) |
||
224 | |||
225 | /** |
||
226 | * USER & LEVEL |
||
227 | * Returns if a level is higher ranking than another level, |
||
228 | * or if a user is higher ranking than another user. |
||
229 | * |
||
230 | 3 | * @param $other |
|
231 | * @return bool |
||
232 | 3 | */ |
|
233 | 3 | public function isHigherThan($other) |
|
237 | |||
238 | /** |
||
239 | * USER & LEVEL |
||
240 | * Returns if a equal is lower ranking than another level, |
||
241 | * or if a user is equal ranking than another user. |
||
242 | * |
||
243 | * @param $other |
||
244 | * @return bool |
||
245 | 1 | */ |
|
246 | public function isEqualTo($other) |
||
250 | |||
251 | |||
252 | /* ------------------------------------------------------------------------------------------------ |
||
253 | | Other Functions |
||
254 | | ------------------------------------------------------------------------------------------------ |
||
255 | */ |
||
256 | /** |
||
257 | * USER & LEVEL |
||
258 | * Return a collection of permissions |
||
259 | * assigned to a user |
||
260 | * |
||
261 | * @return \Illuminate\Support\Collection |
||
262 | */ |
||
263 | protected function cacheGetPermissions() |
||
267 | 6 | ||
268 | 3 | /** |
|
269 | 6 | * USER & LEVEL |
|
270 | * Return a collection of all permissions |
||
271 | 6 | * associated to a user by direct or |
|
272 | * inheritance |
||
273 | 6 | * |
|
274 | * @return \Illuminate\Support\Collection |
||
275 | */ |
||
276 | 6 | protected function cacheGetAllPermissions() |
|
290 | 3 | ||
291 | 1 | /** |
|
292 | 3 | * USER & LEVEL |
|
293 | * Return a collection of permissions that |
||
294 | 3 | * are inherited from a higher level |
|
295 | 2 | * |
|
296 | 3 | * @return \Illuminate\Support\Collection |
|
297 | */ |
||
298 | 3 | protected function cacheGetInheritedPermissions() |
|
314 | |||
315 | 2 | /** |
|
316 | * USER & LEVEL |
||
317 | 2 | * Return a collection of permissions still able to be assigned |
|
318 | * ie, not already inherited or explicitly assigned |
||
319 | * |
||
320 | * @return \Illuminate\Support\Collection |
||
321 | */ |
||
322 | protected function cacheGetAvailablePermissions() |
||
328 | |||
329 | 2 | /** |
|
330 | * USER & LEVEL |
||
331 | 2 | * Check if a model has permission to do something |
|
332 | 1 | * |
|
333 | * @param $permission |
||
334 | 1 | * @return bool |
|
335 | */ |
||
336 | protected function cacheHasPermissionTo($permission) |
||
351 | 23 | ||
352 | /** |
||
353 | 23 | * USER & LEVEL |
|
354 | * Modify a given set of permissions, given the action to be taken |
||
355 | 23 | * |
|
356 | 23 | * @param $permissions |
|
357 | 23 | * @param $action |
|
358 | */ |
||
359 | 23 | protected function modifyPermissions($permissions, $action) |
|
379 | |||
380 | /** |
||
381 | * USER & LEVEL |
||
382 | * Helper function to build the array |
||
383 | * for modifyPermissions |
||
384 | * |
||
385 | 23 | * @param $permissions |
|
386 | * @param $action |
||
387 | 23 | * @return array |
|
388 | * @throws PermissionNotFoundException |
||
389 | 23 | */ |
|
390 | 23 | protected function buildPermissionsArray($permissions, $action) |
|
409 | |||
410 | /** |
||
411 | * USER & LEVEL |
||
412 | 22 | * Helper function to handle adding or negating permissions |
|
413 | * |
||
414 | 22 | * @param $permissionObjects |
|
415 | 12 | */ |
|
416 | protected function addOrNegate($permissionObjects) |
||
424 | |||
425 | /** |
||
426 | * USER & LEVEL |
||
427 | * Helper function to handle removing permissions |
||
428 | 3 | * |
|
429 | * @param $permissionObjects |
||
430 | 3 | */ |
|
431 | 2 | View Code Duplication | protected function remove($permissionObjects) |
442 | |||
443 | /** |
||
444 | * USER & LEVEL |
||
445 | * Help function to handle syncing permissions |
||
446 | * |
||
447 | 2 | * @param $permissionObjects |
|
448 | */ |
||
449 | 2 | View Code Duplication | protected function sync($permissionObjects) |
463 | |||
464 | /** |
||
465 | * USER & LEVEL |
||
466 | * Helper function to get the permission whether it is the permission ID |
||
467 | * or the permission name, or the permission object itself. |
||
468 | * |
||
469 | * @param $permission |
||
470 | * @return \z1haze\Acl\Models\Permission |
||
471 | 23 | * @throws PermissionNotFoundException |
|
472 | */ |
||
473 | 23 | View Code Duplication | protected function getPermission($permission) |
486 | } |
This check looks for methods that are used by a trait but not required by it.
To illustrate, let’s look at the following code example
The trait
Idable
provides a methodequalsId
that in turn relies on the methodgetId()
. If this method does not exist on a class mixing in this trait, the method will fail.Adding the
getId()
as an abstract method to the trait will make sure it is available.