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 ElggPlugin 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 ElggPlugin, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
11 | class ElggPlugin extends \ElggObject { |
||
12 | private $package; |
||
13 | private $manifest; |
||
14 | |||
15 | private $path; |
||
16 | private $errorMsg = ''; |
||
17 | |||
18 | /** |
||
19 | * Set subtype to 'plugin' |
||
20 | * |
||
21 | * @return void |
||
22 | */ |
||
23 | protected function initializeAttributes() { |
||
31 | |||
32 | /** |
||
33 | * Creates a new plugin from path |
||
34 | * |
||
35 | * @note Internal: also supports database objects |
||
36 | * |
||
37 | * @warning Unlike other \ElggEntity objects, you cannot null instantiate |
||
38 | * \ElggPlugin. You must provide the path to the plugin directory. |
||
39 | * |
||
40 | * @param string $path The absolute path of the plugin |
||
41 | * |
||
42 | * @throws PluginException |
||
43 | */ |
||
44 | public function __construct($path) { |
||
87 | |||
88 | /** |
||
89 | * Save the plugin object. Make sure required values exist. |
||
90 | * |
||
91 | * @see \ElggObject::save() |
||
92 | * @return bool |
||
93 | */ |
||
94 | public function save() { |
||
111 | |||
112 | |||
113 | // Plugin ID and path |
||
114 | |||
115 | /** |
||
116 | * Returns the ID (dir name) of this plugin |
||
117 | * |
||
118 | * @return string |
||
119 | */ |
||
120 | public function getID() { |
||
123 | |||
124 | /** |
||
125 | * Returns the manifest's name if available, otherwise the ID. |
||
126 | * |
||
127 | * @return string |
||
128 | * @since 1.8.1 |
||
129 | */ |
||
130 | public function getFriendlyName() { |
||
138 | |||
139 | /** |
||
140 | * Returns the plugin's full path with trailing slash. |
||
141 | * |
||
142 | * @return string |
||
143 | */ |
||
144 | public function getPath() { |
||
147 | |||
148 | /** |
||
149 | * Sets the location of this plugin. |
||
150 | * |
||
151 | * @param string $id The path to the plugin's dir. |
||
152 | * @return bool |
||
153 | */ |
||
154 | public function setID($id) { |
||
157 | |||
158 | /** |
||
159 | * Returns an array of available markdown files for this plugin |
||
160 | * |
||
161 | * @return array |
||
162 | */ |
||
163 | public function getAvailableTextFiles() { |
||
175 | |||
176 | // Load Priority |
||
177 | |||
178 | /** |
||
179 | * Gets the plugin's load priority. |
||
180 | * |
||
181 | * @return int |
||
182 | */ |
||
183 | public function getPriority() { |
||
187 | |||
188 | /** |
||
189 | * Sets the priority of the plugin |
||
190 | * |
||
191 | * @param mixed $priority The priority to set. One of +1, -1, first, last, or a number. |
||
192 | * If given a number, this will displace all plugins at that number |
||
193 | * and set their priorities +1 |
||
194 | * @param mixed $site_guid Optional site GUID. |
||
195 | * @return bool |
||
196 | */ |
||
197 | public function setPriority($priority, $site_guid = null) { |
||
266 | |||
267 | |||
268 | // Plugin settings |
||
269 | |||
270 | /** |
||
271 | * Returns a plugin setting |
||
272 | * |
||
273 | * @param string $name The setting name |
||
274 | * @param mixed $default The default value to return if none is set |
||
275 | * @return mixed |
||
276 | */ |
||
277 | public function getSetting($name, $default = null) { |
||
281 | |||
282 | /** |
||
283 | * Returns an array of all settings saved for this plugin. |
||
284 | * |
||
285 | * @note Unlike user settings, plugin settings are not namespaced. |
||
286 | * |
||
287 | * @return array An array of key/value pairs. |
||
288 | */ |
||
289 | public function getAllSettings() { |
||
317 | |||
318 | /** |
||
319 | * Set a plugin setting for the plugin |
||
320 | * |
||
321 | * @todo This will only work once the plugin has a GUID. |
||
322 | * |
||
323 | * @param string $name The name to set |
||
324 | * @param string $value The value to set |
||
325 | * |
||
326 | * @return bool |
||
327 | */ |
||
328 | public function setSetting($name, $value) { |
||
343 | |||
344 | /** |
||
345 | * Removes a plugin setting name and value. |
||
346 | * |
||
347 | * @param string $name The setting name to remove |
||
348 | * |
||
349 | * @return bool |
||
350 | */ |
||
351 | public function unsetSetting($name) { |
||
354 | |||
355 | /** |
||
356 | * Removes all settings for this plugin. |
||
357 | * |
||
358 | * @todo Should be a better way to do this without dropping to raw SQL. |
||
359 | * @todo If we could namespace the plugin settings this would be cleaner. |
||
360 | * @todo this shouldn't work because ps_prefix will be empty string |
||
361 | * @return bool |
||
362 | */ |
||
363 | public function unsetAllSettings() { |
||
375 | |||
376 | |||
377 | // User settings |
||
378 | |||
379 | /** |
||
380 | * Returns a user's setting for this plugin |
||
381 | * |
||
382 | * @param string $name The setting name |
||
383 | * @param int $user_guid The user GUID |
||
384 | * @param mixed $default The default value to return if none is set |
||
385 | * |
||
386 | * @return mixed The setting string value, the default value or false if there is no user |
||
387 | */ |
||
388 | public function getUserSetting($name, $user_guid = 0, $default = null) { |
||
406 | |||
407 | /** |
||
408 | * Returns an array of all user settings saved for this plugin for the user. |
||
409 | * |
||
410 | * @note Plugin settings are saved with a prefix. This removes that prefix. |
||
411 | * |
||
412 | * @param int $user_guid The user GUID. Defaults to logged in. |
||
413 | * @return array An array of key/value pairs. |
||
414 | */ |
||
415 | public function getAllUserSettings($user_guid = 0) { |
||
453 | |||
454 | /** |
||
455 | * Sets a user setting for a plugin |
||
456 | * |
||
457 | * @param string $name The setting name |
||
458 | * @param string $value The setting value |
||
459 | * @param int $user_guid The user GUID |
||
460 | * |
||
461 | * @return mixed The new setting ID or false |
||
462 | */ |
||
463 | public function setUserSetting($name, $value, $user_guid = 0) { |
||
491 | |||
492 | /** |
||
493 | * Removes a user setting name and value. |
||
494 | * |
||
495 | * @param string $name The user setting name |
||
496 | * @param int $user_guid The user GUID |
||
497 | * @return bool |
||
498 | */ |
||
499 | public function unsetUserSetting($name, $user_guid = 0) { |
||
517 | |||
518 | /** |
||
519 | * Removes all User Settings for this plugin for a particular user |
||
520 | * |
||
521 | * Use {@link removeAllUsersSettings()} to remove all user |
||
522 | * settings for all users. (Note the plural 'Users'.) |
||
523 | * |
||
524 | * @warning 0 does not equal logged in user for this method! |
||
525 | * @todo fix that |
||
526 | * |
||
527 | * @param int $user_guid The user GUID to remove user settings. |
||
528 | * @return bool |
||
529 | */ |
||
530 | View Code Duplication | public function unsetAllUserSettings($user_guid) { |
|
540 | |||
541 | /** |
||
542 | * Removes this plugin's user settings for all users. |
||
543 | * |
||
544 | * Use {@link removeAllUserSettings()} if you just want to remove |
||
545 | * settings for a single user. |
||
546 | * |
||
547 | * @return bool |
||
548 | */ |
||
549 | View Code Duplication | public function unsetAllUsersSettings() { |
|
558 | |||
559 | |||
560 | // validation |
||
561 | |||
562 | /** |
||
563 | * Returns if the plugin is complete, meaning has all required files |
||
564 | * and Elgg can read them and they make sense. |
||
565 | * |
||
566 | * @todo bad name? This could be confused with isValid() from \ElggPluginPackage. |
||
567 | * |
||
568 | * @return bool |
||
569 | */ |
||
570 | public function isValid() { |
||
588 | |||
589 | /** |
||
590 | * Is this plugin active? |
||
591 | * |
||
592 | * @param int $site_guid Optional site guid. |
||
593 | * @return bool |
||
594 | */ |
||
595 | public function isActive($site_guid = null) { |
||
612 | |||
613 | /** |
||
614 | * Checks if this plugin can be activated on the current |
||
615 | * Elgg installation. |
||
616 | * |
||
617 | * @todo remove $site_guid param or implement it |
||
618 | * |
||
619 | * @param mixed $site_guid Optional site guid |
||
620 | * @return bool |
||
621 | */ |
||
622 | public function canActivate($site_guid = null) { |
||
634 | |||
635 | |||
636 | // activating and deactivating |
||
637 | |||
638 | /** |
||
639 | * Actives the plugin for the current site. |
||
640 | * |
||
641 | * @param mixed $site_guid Optional site GUID. |
||
642 | * @return bool |
||
643 | */ |
||
644 | public function activate($site_guid = null) { |
||
687 | |||
688 | /** |
||
689 | * Deactivates the plugin. |
||
690 | * |
||
691 | * @param mixed $site_guid Optional site GUID. |
||
692 | * @return bool |
||
693 | */ |
||
694 | public function deactivate($site_guid = null) { |
||
720 | |||
721 | /** |
||
722 | * Start the plugin. |
||
723 | * |
||
724 | * @param int $flags Start flags for the plugin. See the constants in lib/plugins.php for details. |
||
725 | * @return true |
||
726 | * @throws PluginException |
||
727 | */ |
||
728 | public function start($flags) { |
||
755 | |||
756 | |||
757 | // start helpers |
||
758 | |||
759 | /** |
||
760 | * Get the config object in a deprecation wrapper |
||
761 | * |
||
762 | * @return \Elgg\DeprecationWrapper |
||
763 | */ |
||
764 | protected static function getConfigWrapper() { |
||
773 | |||
774 | /** |
||
775 | * Includes one of the plugins files |
||
776 | * |
||
777 | * @param string $filename The name of the file |
||
778 | * |
||
779 | * @throws PluginException |
||
780 | * @return mixed The return value of the included file (or 1 if there is none) |
||
781 | */ |
||
782 | protected function includeFile($filename) { |
||
799 | |||
800 | /** |
||
801 | * Checks whether a plugin file with the given name exists |
||
802 | * |
||
803 | * @param string $filename The name of the file |
||
804 | * @return bool |
||
805 | */ |
||
806 | protected function canReadFile($filename) { |
||
809 | |||
810 | /** |
||
811 | * Registers the plugin's views |
||
812 | * |
||
813 | * @throws PluginException |
||
814 | * @return void |
||
815 | */ |
||
816 | protected function registerViews() { |
||
823 | |||
824 | /** |
||
825 | * Registers the plugin's languages |
||
826 | * |
||
827 | * @throws PluginException |
||
828 | * @return true |
||
829 | */ |
||
830 | protected function registerLanguages() { |
||
847 | |||
848 | /** |
||
849 | * Registers the plugin's classes |
||
850 | * |
||
851 | * @throws PluginException |
||
852 | * @return true |
||
853 | */ |
||
854 | protected function registerClasses() { |
||
863 | |||
864 | /** |
||
865 | * Get an attribute or private setting value |
||
866 | * |
||
867 | * @param string $name Name of the attribute or private setting |
||
868 | * @return mixed |
||
869 | */ |
||
870 | public function __get($name) { |
||
896 | |||
897 | /** |
||
898 | * Get a value from private settings. |
||
899 | * |
||
900 | * @param string $name Name |
||
901 | * @return mixed |
||
902 | * @deprecated 1.9 |
||
903 | */ |
||
904 | public function get($name) { |
||
908 | |||
909 | /** |
||
910 | * Set a value as private setting or attribute. |
||
911 | * |
||
912 | * Attributes include title and description. |
||
913 | * |
||
914 | * @param string $name Name of the attribute or private_setting |
||
915 | * @param mixed $value Value to be set |
||
916 | * @return void |
||
917 | */ |
||
918 | View Code Duplication | public function __set($name, $value) { |
|
931 | |||
932 | /** |
||
933 | * Save a value as private setting or attribute. |
||
934 | * |
||
935 | * Attributes include title and description. |
||
936 | * |
||
937 | * @param string $name Name |
||
938 | * @param mixed $value Value |
||
939 | * @return bool |
||
940 | */ |
||
941 | public function set($name, $value) { |
||
947 | |||
948 | /** |
||
949 | * Sets the plugin to active or inactive for $site_guid. |
||
950 | * |
||
951 | * @param bool $active Set to active or inactive |
||
952 | * @param mixed $site_guid Int for specific site, null for current site. |
||
953 | * |
||
954 | * @return bool |
||
955 | */ |
||
956 | private function setStatus($active, $site_guid = null) { |
||
981 | |||
982 | /** |
||
983 | * Returns the last error message registered. |
||
984 | * |
||
985 | * @return string|null |
||
986 | */ |
||
987 | public function getError() { |
||
990 | |||
991 | /** |
||
992 | * Returns this plugin's \ElggPluginManifest object |
||
993 | * |
||
994 | * @return \ElggPluginManifest |
||
995 | */ |
||
996 | public function getManifest() { |
||
1010 | |||
1011 | /** |
||
1012 | * Returns this plugin's \ElggPluginPackage object |
||
1013 | * |
||
1014 | * @return \ElggPluginPackage |
||
1015 | */ |
||
1016 | public function getPackage() { |
||
1030 | } |
||
1031 |
It seems like the type of the argument is not accepted by the function/method which you are calling.
In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug.
We suggest to add an explicit type cast like in the following example: