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 UserCapabilities 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 UserCapabilities, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
23 | class UserCapabilities { |
||
24 | |||
25 | /** |
||
26 | * @var PluginHooksService $hooks |
||
27 | */ |
||
28 | private $hooks; |
||
29 | |||
30 | /** |
||
31 | * @var EntityTable |
||
32 | */ |
||
33 | private $entities; |
||
34 | |||
35 | /** |
||
36 | * @var ElggSession |
||
37 | */ |
||
38 | private $session; |
||
39 | |||
40 | /** |
||
41 | * Constructor |
||
42 | * |
||
43 | * @param PluginHooksService $hooks Plugin hooks service |
||
44 | * @param EntityTable $entities Entity table |
||
45 | * @param ElggSession $session Session |
||
46 | */ |
||
47 | 198 | public function __construct(PluginHooksService $hooks, EntityTable $entities, ElggSession $session) { |
|
52 | |||
53 | /** |
||
54 | * Decides if the access system should be ignored for a user. |
||
55 | * |
||
56 | * Returns true (meaning ignore access) if either of these 2 conditions are true: |
||
57 | * 1) an admin user guid is passed to this function. |
||
58 | * 2) {@link elgg_get_ignore_access()} returns true. |
||
59 | * |
||
60 | * @see elgg_set_ignore_access() |
||
61 | * |
||
62 | * @param int $user_guid The user to check against. |
||
63 | * |
||
64 | * @return bool |
||
65 | */ |
||
66 | 262 | public function canBypassPermissionsCheck($user_guid = 0) { |
|
81 | |||
82 | /** |
||
83 | * Can a user edit this entity? |
||
84 | * |
||
85 | * @tip Can be overridden by registering for the permissions_check plugin hook. |
||
86 | * |
||
87 | * @param ElggEntity $entity Object entity |
||
88 | * @param int $user_guid The user GUID, optionally (default: logged in user) |
||
89 | * |
||
90 | * @return bool Whether this entity is editable by the given user. |
||
91 | * @see elgg_set_ignore_access() |
||
92 | */ |
||
93 | 23 | public function canEdit(ElggEntity $entity, $user_guid = 0) { |
|
138 | |||
139 | /** |
||
140 | * Can a user delete this entity? |
||
141 | * |
||
142 | * @tip Can be overridden by registering for the permissions_check:delete plugin hook. |
||
143 | * |
||
144 | * @param ElggEntity $entity Object entity |
||
145 | * @param int $user_guid The user GUID, optionally (default: logged in user) |
||
146 | * |
||
147 | * @return bool Whether this entity is deletable by the given user. |
||
148 | * @since 1.11 |
||
149 | * @see elgg_set_ignore_access() |
||
150 | */ |
||
151 | 4 | public function canDelete(ElggEntity $entity, $user_guid = 0) { |
|
170 | |||
171 | /** |
||
172 | * Can a user delete this river item? |
||
173 | * |
||
174 | * @tip Can be overridden by registering for the "permissions_check:delete", "river" plugin hook. |
||
175 | * |
||
176 | * @param ElggRiverItem $item River item |
||
177 | * @param int $user_guid The user GUID, optionally (default: logged in user) |
||
178 | * |
||
179 | * @return bool Whether this river item should be considered deletable by the given user. |
||
180 | * @since 2.3 |
||
181 | * @see elgg_set_ignore_access() |
||
182 | */ |
||
183 | View Code Duplication | public function canDeleteRiverItem(ElggRiverItem $item, $user_guid = 0) { |
|
200 | |||
201 | /** |
||
202 | * Can a user edit metadata on this entity? |
||
203 | * |
||
204 | * If no specific metadata is passed, it returns whether the user can |
||
205 | * edit any metadata on the entity. |
||
206 | * |
||
207 | * @tip Can be overridden by by registering for the permissions_check:metadata |
||
208 | * plugin hook. |
||
209 | * |
||
210 | * @param ElggEntity $entity Object entity |
||
211 | * @param int $user_guid The user GUID, optionally (default: logged in user) |
||
212 | * @param ElggMetadata $metadata The piece of metadata to specifically check or null for any metadata |
||
213 | * |
||
214 | * @return bool |
||
215 | * @see elgg_set_ignore_access() |
||
216 | */ |
||
217 | 8 | public function canEditMetadata(ElggEntity $entity, $user_guid = 0, ElggMetadata $metadata = null) { |
|
252 | |||
253 | /** |
||
254 | * Determines whether or not the user can edit this annotation |
||
255 | * |
||
256 | * @param Elggentity $entity Object entity |
||
257 | * @param int $user_guid The GUID of the user (defaults to currently logged in user) |
||
258 | * @param ElggAnnotation $annotation Annotation |
||
259 | * |
||
260 | * @return bool |
||
261 | * @see elgg_set_ignore_access() |
||
262 | */ |
||
263 | 5 | public function canEditAnnotation(ElggEntity $entity, $user_guid = 0, ElggAnnotation $annotation = null) { |
|
301 | |||
302 | /** |
||
303 | * Can a user add an entity to this container |
||
304 | * |
||
305 | * @param ElggEntity $entity Container entity |
||
306 | * @param int $user_guid The GUID of the user creating the entity (0 for logged in user). |
||
307 | * @param string $type The type of entity we're looking to write |
||
308 | * @param string $subtype The subtype of the entity we're looking to write |
||
309 | * |
||
310 | * @return bool |
||
311 | * @see elgg_set_ignore_access() |
||
312 | */ |
||
313 | 6 | public function canWriteToContainer(ElggEntity $entity, $user_guid = 0, $type = 'all', $subtype = 'all') { |
|
360 | |||
361 | /** |
||
362 | * Can a user comment on an entity? |
||
363 | * |
||
364 | * @tip Can be overridden by registering for the permissions_check:comment, |
||
365 | * <entity type> plugin hook. |
||
366 | * |
||
367 | * @param ElggEntity $entity Object entity |
||
368 | * @param int $user_guid User guid (default is logged in user) |
||
369 | * @param bool $default Default permission |
||
370 | * @return bool |
||
371 | */ |
||
372 | 5 | View Code Duplication | public function canComment(ElggEntity $entity, $user_guid = 0, $default = null) { |
391 | |||
392 | /** |
||
393 | * Can a user annotate an entity? |
||
394 | * |
||
395 | * @tip Can be overridden by registering for the plugin hook [permissions_check:annotate:<name>, |
||
396 | * <entity type>] or [permissions_check:annotate, <entity type>]. The hooks are called in that order. |
||
397 | * |
||
398 | * @tip If you want logged out users to annotate an object, do not call |
||
399 | * canAnnotate(). It's easier than using the plugin hook. |
||
400 | * |
||
401 | * @param ElggEntity $entity Objet entity |
||
402 | * @param int $user_guid User guid (default is logged in user) |
||
403 | * @param string $annotation_name The name of the annotation (default is unspecified) |
||
404 | * |
||
405 | * @return bool |
||
406 | */ |
||
407 | 8 | public function canAnnotate(ElggEntity $entity, $user_guid = 0, $annotation_name = '') { |
|
439 | |||
440 | /** |
||
441 | * Can a user download a file? |
||
442 | * |
||
443 | * @tip Can be overridden by registering for the permissions_check:download,file plugin hook. |
||
444 | * |
||
445 | * @param ElggFile $entity File entity |
||
446 | * @param int $user_guid User guid (default is logged in user) |
||
447 | * @param bool $default Default permission |
||
448 | * |
||
449 | * @return bool |
||
450 | */ |
||
451 | 2 | View Code Duplication | public function canDownload(ElggFile $entity, $user_guid = 0, $default = true) { |
469 | |||
470 | } |
||
471 |
It seems like the method you are trying to call exists only in some of the possible types.
Let’s take a look at an example:
Available Fixes
Add an additional type-check:
Only allow a single type to be passed if the variable comes from a parameter: