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 ZervWizard 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 ZervWizard, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
27 | class ZervWizard |
||
|
|||
28 | { |
||
29 | // whether or not all steps of the form are complete |
||
30 | public $_complete = false; |
||
31 | |||
32 | // internal array to store the various steps |
||
33 | public $_steps = array(); |
||
34 | |||
35 | // the current step |
||
36 | public $_currentStep = null; |
||
37 | |||
38 | // the prefix of the container key where form values are stored |
||
39 | public $_containerPrefix = '__wiz_'; |
||
40 | |||
41 | // an array of any errors that have occurred |
||
42 | public $_errors = array(); |
||
43 | |||
44 | // key in container where step status is stored |
||
45 | public $_step_status_key = '__step_complete'; |
||
46 | |||
47 | // key in container where expected action is stored |
||
48 | public $_step_expected_key = '__expected_action'; |
||
49 | |||
50 | // options to use for the wizard |
||
51 | public $options = array('redirectAfterPost' => true); |
||
52 | |||
53 | // action that resets the container |
||
54 | public $resetAction = '__reset'; |
||
55 | |||
56 | /** |
||
57 | * ZervWizard |
||
58 | * |
||
59 | * Constructor. Primarily sets up the container |
||
60 | * |
||
61 | * @param array &$container Reference to container array |
||
62 | * @param string $name A unique name for the wizard for container storage |
||
63 | */ |
||
64 | public function __construct($container, $name) |
||
84 | |||
85 | /** |
||
86 | * process |
||
87 | * |
||
88 | * Processes the form for the specified step. If the processed step |
||
89 | * is complete, then the wizard is set to use the next step. If this |
||
90 | * is the initial call to process, then the wizard is set to use the |
||
91 | * first step. Once the next step is determined, the prepare method |
||
92 | * is called for the step. This has the method name prepare_[step name]() |
||
93 | * |
||
94 | * @todo Need a way to jump between steps, e.g. from step 2 to 4 and validating all data |
||
95 | * |
||
96 | * @param string $action The step being processed. This should correspond |
||
97 | * to a step created in addStep() |
||
98 | * @param array &$form The unmodified form values to process |
||
99 | * @param bool $process True if the step is being processed, false if being prepared |
||
100 | */ |
||
101 | public function process($action, $form, $process = true) |
||
181 | |||
182 | /** |
||
183 | * completeCallback |
||
184 | * |
||
185 | * Function to run once the final step has been processed and is valid. |
||
186 | * This should be overwritten in child classes |
||
187 | */ |
||
188 | public function completeCallback() |
||
191 | |||
192 | public function doRedirect() |
||
201 | |||
202 | /** |
||
203 | * isComplete |
||
204 | * |
||
205 | * Check if the form is complete. This can only be properly determined |
||
206 | * after process() has been called. |
||
207 | * |
||
208 | * @return bool True if the form is complete and valid, false if not |
||
209 | */ |
||
210 | public function isComplete() |
||
214 | |||
215 | /** |
||
216 | * setCurrentStep |
||
217 | * |
||
218 | * Sets the current step in the form. This should generally only be |
||
219 | * called internally but you may have reason to change the current |
||
220 | * step. |
||
221 | * |
||
222 | * @param string $step The step to set as current |
||
223 | */ |
||
224 | public function setCurrentStep($step) |
||
234 | |||
235 | /** |
||
236 | * @return mixed|null |
||
237 | */ |
||
238 | public function getExpectedStep() |
||
247 | |||
248 | /** |
||
249 | * stepExists |
||
250 | * |
||
251 | * Check if the given step exists |
||
252 | * |
||
253 | * @param string $stepname The name of the step to check for |
||
254 | * |
||
255 | * @return bool True if the step exists, false if not |
||
256 | */ |
||
257 | public function stepExists($stepname) |
||
261 | |||
262 | /** |
||
263 | * getStepName |
||
264 | * |
||
265 | * Get the name of the current step |
||
266 | * |
||
267 | * @return string The name of the current step |
||
268 | */ |
||
269 | public function getStepName() |
||
273 | |||
274 | /** |
||
275 | * getStepNumber |
||
276 | * |
||
277 | * Gets the step number (from 1 to N where N is the number of steps |
||
278 | * in the wizard) of the current step |
||
279 | * |
||
280 | * @param string $step Optional. The step to get the number for. If null then uses current step |
||
281 | * |
||
282 | * @return int The number of the step. 0 if something went wrong |
||
283 | */ |
||
284 | public function getStepNumber($step = null) |
||
302 | |||
303 | /** |
||
304 | * @param $step |
||
305 | * |
||
306 | * @return bool |
||
307 | */ |
||
308 | public function stepCanBeProcessed($step) |
||
326 | |||
327 | /** |
||
328 | * getStepProperty |
||
329 | * |
||
330 | * Retrieve a property for a given step. At this stage, the only |
||
331 | * property steps have is a title property. |
||
332 | * |
||
333 | * @param string $key The key to get a property for |
||
334 | * @param mixed $default The value to return if the key isn't found |
||
335 | * |
||
336 | * @return mixed The property value or the default value |
||
337 | */ |
||
338 | public function getStepProperty($key, $default = null) |
||
347 | |||
348 | /** |
||
349 | * getFirstStep |
||
350 | * |
||
351 | * Get the step name of the first step |
||
352 | * |
||
353 | * @return string The name of the first step, or null if no steps |
||
354 | */ |
||
355 | public function getFirstStep() |
||
361 | |||
362 | /** |
||
363 | * @return null |
||
364 | */ |
||
365 | public function getFirstIncompleteStep() |
||
380 | |||
381 | /** |
||
382 | * getPreviousStep |
||
383 | * |
||
384 | * Gets the step name of the previous step. If the current |
||
385 | * step is the first step, then null is returned |
||
386 | * |
||
387 | * @param $step |
||
388 | * |
||
389 | * @return string The name of the previous step, or null |
||
390 | */ |
||
391 | View Code Duplication | public function getPreviousStep($step) |
|
407 | |||
408 | /** |
||
409 | * getFollowingStep |
||
410 | * |
||
411 | * Get the step name of the next step. If the current |
||
412 | * step is the last step, returns null |
||
413 | * |
||
414 | * @param $step |
||
415 | * |
||
416 | * @return string The name of the next step, or null |
||
417 | */ |
||
418 | View Code Duplication | public function getFollowingStep($step) |
|
437 | |||
438 | /** |
||
439 | * addStep |
||
440 | * |
||
441 | * Adds a step to the wizard |
||
442 | * |
||
443 | * @param string $stepname The name of the step |
||
444 | * @param string $title The title of the current step |
||
445 | */ |
||
446 | public function addStep($stepname, $title) |
||
464 | |||
465 | /** |
||
466 | * isFirstStep |
||
467 | * |
||
468 | * Check if the current step is the first step |
||
469 | * |
||
470 | * @return bool True if the current step is the first step |
||
471 | */ |
||
472 | public function isFirstStep() |
||
478 | |||
479 | /** |
||
480 | * isLastStep |
||
481 | * |
||
482 | * Check if the current step is the last step |
||
483 | * |
||
484 | * @return bool True if the current step is the last step |
||
485 | */ |
||
486 | public function isLastStep() |
||
492 | |||
493 | /** |
||
494 | * setValue |
||
495 | * |
||
496 | * Sets a value in the container |
||
497 | * |
||
498 | * @param string $key The key for the value to set |
||
499 | * @param mixed $val The value |
||
500 | */ |
||
501 | public function setValue($key, $val) |
||
505 | |||
506 | /** |
||
507 | * getValue |
||
508 | * |
||
509 | * Gets a value from the container |
||
510 | * |
||
511 | * @param string $key The key for the value to get |
||
512 | * @param mixed $default The value to return if the key doesn't exist |
||
513 | * |
||
514 | * @return mixed Either the key's value or the default value |
||
515 | */ |
||
516 | public function getValue($key, $default = null) |
||
520 | |||
521 | /** |
||
522 | * clearContainer |
||
523 | * |
||
524 | * Removes all data from the container. This is primarily used |
||
525 | * to reset the wizard data completely |
||
526 | */ |
||
527 | public function clearContainer() |
||
533 | |||
534 | /** |
||
535 | * coalesce |
||
536 | * |
||
537 | * Initializes a variable, by returning either the variable |
||
538 | * or a default value |
||
539 | * |
||
540 | * @param mixed &$var The variable to fetch |
||
541 | * @param mixed $default The value to return if variable doesn't exist or is null |
||
542 | * |
||
543 | * @return mixed The variable value or the default value |
||
544 | */ |
||
545 | public function coalesce(&$var, $default = null) |
||
549 | |||
550 | /** |
||
551 | * addError |
||
552 | * |
||
553 | * Add an error |
||
554 | * |
||
555 | * @param string $key An identifier for the error (e.g. the field name) |
||
556 | * @param string $val An error message |
||
557 | */ |
||
558 | public function addError($key, $val) |
||
562 | |||
563 | /** |
||
564 | * isError |
||
565 | * |
||
566 | * Check if an error has occurred |
||
567 | * |
||
568 | * @param string $key The field to check for error. If none specified checks for any error |
||
569 | * |
||
570 | * @return bool True if an error has occurred, false if not |
||
571 | */ |
||
572 | public function isError($key = null) |
||
580 | |||
581 | /** |
||
582 | * @param $key |
||
583 | * |
||
584 | * @return null |
||
585 | */ |
||
586 | public function getError($key) |
||
590 | } |
||
591 |
You can fix this by adding a namespace to your class:
When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries.