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 EE_Admin_Hooks 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 EE_Admin_Hooks, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 12 | abstract class EE_Admin_Hooks extends EE_Base |
||
| 13 | { |
||
| 14 | |||
| 15 | |||
| 16 | /** |
||
| 17 | * we're just going to use this to hold the name of the caller class (child class name) |
||
| 18 | * |
||
| 19 | * @var string |
||
| 20 | */ |
||
| 21 | public $caller; |
||
| 22 | |||
| 23 | |||
| 24 | /** |
||
| 25 | * this is just a flag set automatically to indicate whether we've got an extended hook class running (i.e. |
||
| 26 | * espresso_events_Registration_Form_Hooks_Extend extends espresso_events_Registration_Form_Hooks). This flag is |
||
| 27 | * used later to make sure we require the needed files. |
||
| 28 | * |
||
| 29 | * @var bool |
||
| 30 | */ |
||
| 31 | protected $_extend; |
||
| 32 | |||
| 33 | |||
| 34 | /** |
||
| 35 | * child classes MUST set this property so that the page object can be loaded correctly |
||
| 36 | * |
||
| 37 | * @var string |
||
| 38 | */ |
||
| 39 | protected $_name; |
||
| 40 | |||
| 41 | |||
| 42 | /** |
||
| 43 | * This is set by child classes and is an associative array of ajax hooks in the format: |
||
| 44 | * array( |
||
| 45 | * 'ajax_action_ref' => 'executing_method'; //must be public |
||
| 46 | * ) |
||
| 47 | * |
||
| 48 | * @var array |
||
| 49 | */ |
||
| 50 | protected $_ajax_func; |
||
| 51 | |||
| 52 | |||
| 53 | /** |
||
| 54 | * This is an array of methods that get executed on a page routes admin_init hook. Use the following format: |
||
| 55 | * array( |
||
| 56 | * 'page_route' => 'executing_method' //must be public |
||
| 57 | * ) |
||
| 58 | * |
||
| 59 | * @var array |
||
| 60 | */ |
||
| 61 | protected $_init_func; |
||
| 62 | |||
| 63 | |||
| 64 | /** |
||
| 65 | * This is an array of methods that output metabox content for the given page route. Use the following format: |
||
| 66 | * array( |
||
| 67 | * 0 => array( |
||
| 68 | * 'page_route' => 'string_for_page_route', //must correspond to a page route in the class being connected |
||
| 69 | * with (i.e. "edit_event") If this is in an array then the same params below will be used but the metabox |
||
| 70 | * will be added to each route. |
||
| 71 | * 'func' => 'executing_method', //must be public (i.e. public function executing_method($post, |
||
| 72 | * $callback_args){} ). Note if you include callback args in the array then you need to declare them in the |
||
| 73 | * method arguments. |
||
| 74 | * 'id' => 'identifier_for_metabox', //so it can be removed by addons (optional, class will set it |
||
| 75 | * automatically) |
||
| 76 | * 'priority' => 'default', //default 'default' (optional) |
||
| 77 | * 'label' => __('Localized Title', 'event_espresso'), |
||
| 78 | * 'context' => 'advanced' //advanced is default (optional), |
||
| 79 | * 'callback_args' => array() //any callback args to include (optional) |
||
| 80 | * ) |
||
| 81 | * Why are we indexing numerically? Because it's possible there may be more than one metabox per page_route. |
||
| 82 | * |
||
| 83 | * @var array |
||
| 84 | */ |
||
| 85 | protected $_metaboxes; |
||
| 86 | |||
| 87 | |||
| 88 | /** |
||
| 89 | * This is an array of values that indicate any metaboxes we want removed from a given page route. Usually this is |
||
| 90 | * used when caffeinated functionality is replacing decaffeinated functionality. Use the following format for the |
||
| 91 | * array: array( |
||
| 92 | * 0 => array( |
||
| 93 | * 'page_route' => 'string_for_page_route' //can be string or array of strings that match a page_route(s) |
||
| 94 | * that are in the class being connected with (i.e. 'edit', or 'create_new'). |
||
| 95 | * 'id' => 'identifier_for_metabox', //what the id is of the metabox being removed |
||
| 96 | * 'context' => 'normal', //the context for the metabox being removed (has to match) |
||
| 97 | * 'screen' => 'screen_id', //(optional), if not included then this class will attempt to remove the metabox |
||
| 98 | * using the currently loaded screen object->id however, there may be cases where you have to specify the |
||
| 99 | * id for the screen the metabox is on. |
||
| 100 | * ) |
||
| 101 | * ) |
||
| 102 | * |
||
| 103 | * @var array |
||
| 104 | */ |
||
| 105 | protected $_remove_metaboxes; |
||
| 106 | |||
| 107 | |||
| 108 | /** |
||
| 109 | * This parent class takes care of loading the scripts and styles if the child class has set the properties for |
||
| 110 | * them in the following format. Note, the first array index ('register') is for defining all the registers. The |
||
| 111 | * second array index is for indicating what routes each script/style loads on. array( |
||
| 112 | * 'registers' => array( |
||
| 113 | * 'script_ref' => array( // if more than one script is to be loaded its best to use the 'dependency' |
||
| 114 | * argument to link scripts together. |
||
| 115 | * 'type' => 'js' // 'js' or 'css' (defaults to js). This tells us what type of wp_function to use |
||
| 116 | * 'url' => 'http://urltoscript.css.js', |
||
| 117 | * 'depends' => array('jquery'), //an array of dependencies for the scripts. REMEMBER, if a script has |
||
| 118 | * already been registered elsewhere in the system. You can just use the depends array to make sure it |
||
| 119 | * gets loaded before the one you are setting here. |
||
| 120 | * 'footer' => TRUE //defaults to true (styles don't use this parameter) |
||
| 121 | * ), |
||
| 122 | * 'enqueues' => array( //this time each key corresponds to the script ref followed by an array of page routes |
||
| 123 | * the script gets enqueued on. |
||
| 124 | * 'script_ref' => array('route_one', 'route_two') |
||
| 125 | * ), |
||
| 126 | * 'localize' => array( //this allows you to set a localize object. Indicate which script the object is being |
||
| 127 | * attached to and then include an array indexed by the name of the object and the array of key/value pairs for |
||
| 128 | * the object. |
||
| 129 | * 'scrip_ref' => array( |
||
| 130 | * 'NAME_OF_JS_OBJECT' => array( |
||
| 131 | * 'translate_ref' => __('localized_string', 'event_espresso'), |
||
| 132 | * 'some_data' => 5 |
||
| 133 | * ) |
||
| 134 | * ) |
||
| 135 | * ) |
||
| 136 | * ) |
||
| 137 | * |
||
| 138 | * @var array |
||
| 139 | */ |
||
| 140 | protected $_scripts_styles; |
||
| 141 | |||
| 142 | |||
| 143 | /** |
||
| 144 | * This is a property that will contain the current route. |
||
| 145 | * |
||
| 146 | * @var string; |
||
| 147 | */ |
||
| 148 | protected $_current_route; |
||
| 149 | |||
| 150 | |||
| 151 | /** |
||
| 152 | * this optional property can be set by child classes to override the priority for the automatic action/filter hook |
||
| 153 | * loading in the `_load_routed_hooks()` method. Please follow this format: array( |
||
| 154 | * 'wp_hook_reference' => 1 |
||
| 155 | * ) |
||
| 156 | * ) |
||
| 157 | * |
||
| 158 | * @var array |
||
| 159 | */ |
||
| 160 | protected $_wp_action_filters_priority; |
||
| 161 | |||
| 162 | |||
| 163 | /** |
||
| 164 | * This just holds a merged array of the $_POST and $_GET vars in favor of $_POST |
||
| 165 | * |
||
| 166 | * @var array |
||
| 167 | */ |
||
| 168 | protected $_req_data; |
||
| 169 | |||
| 170 | |||
| 171 | /** |
||
| 172 | * This just holds an instance of the page object for this hook |
||
| 173 | * |
||
| 174 | * @var EE_Admin_Page |
||
| 175 | */ |
||
| 176 | protected $_page_object; |
||
| 177 | |||
| 178 | |||
| 179 | /** |
||
| 180 | * This holds the EE_Admin_Page object from the calling admin page that this object hooks into. |
||
| 181 | * |
||
| 182 | * @var EE_Admin_Page|EE_Admin_Page_CPT |
||
| 183 | */ |
||
| 184 | protected $_adminpage_obj; |
||
| 185 | |||
| 186 | |||
| 187 | /** |
||
| 188 | * Holds EE_Registry object |
||
| 189 | * |
||
| 190 | * @var EE_Registry |
||
| 191 | */ |
||
| 192 | protected $EE = null; |
||
| 193 | |||
| 194 | |||
| 195 | /** |
||
| 196 | * constructor |
||
| 197 | * |
||
| 198 | * @param EE_Admin_Page $admin_page the calling admin_page_object |
||
|
|
|||
| 199 | */ |
||
| 200 | public function __construct(EE_Admin_Page $adminpage) |
||
| 201 | { |
||
| 202 | |||
| 203 | $this->_adminpage_obj = $adminpage; |
||
| 204 | $this->_req_data = array_merge($_GET, $_POST); |
||
| 205 | $this->_set_defaults(); |
||
| 206 | $this->_set_hooks_properties(); |
||
| 207 | // first let's verify we're on the right page |
||
| 208 | if (! isset($this->_req_data['page']) |
||
| 209 | || (isset($this->_req_data['page']) |
||
| 210 | && $this->_adminpage_obj->page_slug |
||
| 211 | != $this->_req_data['page'])) { |
||
| 212 | return; |
||
| 213 | } //get out nothing more to be done here. |
||
| 214 | // allow for extends to modify properties |
||
| 215 | if (method_exists($this, '_extend_properties')) { |
||
| 216 | $this->_extend_properties(); |
||
| 217 | } |
||
| 218 | $this->_set_page_object(); |
||
| 219 | $this->_init_hooks(); |
||
| 220 | $this->_load_custom_methods(); |
||
| 221 | $this->_load_routed_hooks(); |
||
| 222 | add_action('admin_enqueue_scripts', array($this, 'enqueue_scripts_styles')); |
||
| 223 | add_action('admin_enqueue_scripts', array($this, 'add_metaboxes'), 20); |
||
| 224 | add_action('admin_enqueue_scripts', array($this, 'remove_metaboxes'), 15); |
||
| 225 | $this->_ajax_hooks(); |
||
| 226 | } |
||
| 227 | |||
| 228 | |||
| 229 | /** |
||
| 230 | * used by child classes to set the following properties: |
||
| 231 | * $_ajax_func (optional) |
||
| 232 | * $_init_func (optional) |
||
| 233 | * $_metaboxes (optional) |
||
| 234 | * $_scripts (optional) |
||
| 235 | * $_styles (optional) |
||
| 236 | * $_name (required) |
||
| 237 | * Also in this method will be registered any scripts or styles loaded on the targeted page (as indicated in the |
||
| 238 | * _scripts/_styles properties) Also children should place in this method any filters/actions that have to happen |
||
| 239 | * really early on page load (just after admin_init) if they want to have them registered for handling early. |
||
| 240 | * |
||
| 241 | * @access protected |
||
| 242 | * @abstract |
||
| 243 | * @return void |
||
| 244 | */ |
||
| 245 | abstract protected function _set_hooks_properties(); |
||
| 246 | |||
| 247 | |||
| 248 | /** |
||
| 249 | * The hooks for enqueue_scripts and enqueue_styles will be run in here. Child classes need to define their |
||
| 250 | * scripts and styles in the relevant $_scripts and $_styles properties. Child classes must have also already |
||
| 251 | * registered the scripts and styles using wp_register_script and wp_register_style functions. |
||
| 252 | * |
||
| 253 | * @access public |
||
| 254 | * @return void |
||
| 255 | */ |
||
| 256 | public function enqueue_scripts_styles() |
||
| 350 | |||
| 351 | |||
| 352 | /** |
||
| 353 | * just set the defaults for the hooks properties. |
||
| 354 | * |
||
| 355 | * @access private |
||
| 356 | * @return void |
||
| 357 | */ |
||
| 358 | private function _set_defaults() |
||
| 365 | |||
| 366 | |||
| 367 | /** |
||
| 368 | * A helper for determining the current route. |
||
| 369 | * @return string |
||
| 370 | */ |
||
| 371 | private function getCurrentRoute() |
||
| 379 | |||
| 380 | |||
| 381 | /** |
||
| 382 | * this sets the _page_object property |
||
| 383 | * |
||
| 384 | * @access protected |
||
| 385 | * @return void |
||
| 386 | */ |
||
| 387 | protected function _set_page_object() |
||
| 426 | |||
| 427 | |||
| 428 | /** |
||
| 429 | * Child "hook" classes can declare any methods that they want executed when a specific page route is loaded. The |
||
| 430 | * advantage of this is when doing things like running our own db interactions on saves etc. Remember that |
||
| 431 | * $this->_req_data (all the _POST and _GET data) is available to your methods. |
||
| 432 | * |
||
| 433 | * @access private |
||
| 434 | * @return void |
||
| 435 | */ |
||
| 436 | private function _load_custom_methods() |
||
| 481 | |||
| 482 | |||
| 483 | /** |
||
| 484 | * This method will search for a corresponding method with a name matching the route and the wp_hook to run. This |
||
| 485 | * allows child hook classes to target hooking into a specific wp action or filter hook ONLY on a certain route. |
||
| 486 | * just remember, methods MUST be public Future hooks should be added in here to be access by child classes. |
||
| 487 | * |
||
| 488 | * @return void |
||
| 489 | */ |
||
| 490 | private function _load_routed_hooks() |
||
| 544 | |||
| 545 | |||
| 546 | /** |
||
| 547 | * Loop throught the $_ajax_func array and add_actions for the array. |
||
| 548 | * |
||
| 549 | * @return void |
||
| 550 | */ |
||
| 551 | private function _ajax_hooks() |
||
| 577 | |||
| 578 | |||
| 579 | /** |
||
| 580 | * Loop throught the $_init_func array and add_actions for the array. |
||
| 581 | * |
||
| 582 | * @return void |
||
| 583 | */ |
||
| 584 | protected function _init_hooks() |
||
| 613 | |||
| 614 | |||
| 615 | /** |
||
| 616 | * Loop through the _metaboxes property and add_metaboxes accordingly |
||
| 617 | * //todo we could eventually make this a config component class (i.e. new EE_Metabox); |
||
| 618 | * |
||
| 619 | * @access public |
||
| 620 | * @return void |
||
| 621 | */ |
||
| 622 | public function add_metaboxes() |
||
| 629 | |||
| 630 | |||
| 631 | private function _handle_metabox_array($boxes, $add = true) |
||
| 652 | |||
| 653 | |||
| 654 | /** |
||
| 655 | * Loop through the _remove_metaboxes property and remove metaboxes accordingly. |
||
| 656 | * |
||
| 657 | * @access public |
||
| 658 | * @return void |
||
| 659 | */ |
||
| 660 | public function remove_metaboxes() |
||
| 668 | |||
| 669 | |||
| 670 | /** |
||
| 671 | * This just handles adding a metabox |
||
| 672 | * |
||
| 673 | * @access private |
||
| 674 | * @param array $args an array of args that have been set for this metabox by the child class |
||
| 675 | */ |
||
| 676 | private function _add_metabox($args) |
||
| 709 | |||
| 710 | |||
| 711 | private function _remove_metabox($args) |
||
| 734 | } |
||
| 735 |
This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function. It has, however, found a similar but not annotated parameter which might be a good fit.
Consider the following example. The parameter
$irelandis not defined by the methodfinale(...).The most likely cause is that the parameter was changed, but the annotation was not.