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
$ireland
is not defined by the methodfinale(...)
.The most likely cause is that the parameter was changed, but the annotation was not.