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 StateMachineBehavior 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 StateMachineBehavior, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
14 | class StateMachineBehavior extends ModelBehavior { |
||
15 | |||
16 | /** |
||
17 | * Allows us to support writing both: is('parked') and isParked() |
||
18 | * |
||
19 | * @var array |
||
20 | */ |
||
21 | public $mapMethods = array( |
||
22 | '/when([A-Z][a-zA-Z0-9]+)/' => 'when', |
||
23 | '/on([A-Z][a-zA-Z0-9]+)/' => 'on' |
||
24 | ); |
||
25 | |||
26 | protected $_defaultSettings = array( |
||
27 | 'transition_listeners' => array( |
||
28 | 'transition' => array( |
||
29 | 'before' => array(), |
||
30 | 'after' => array() |
||
31 | ) |
||
32 | ), |
||
33 | 'state_listeners' => array(), |
||
34 | 'methods' => array() |
||
35 | ); |
||
36 | |||
37 | /** |
||
38 | * Array of all configured states. Initialized by self::setup() |
||
39 | * @var array |
||
40 | */ |
||
41 | protected $_availableStates = array(); |
||
42 | |||
43 | /** |
||
44 | * Adds a available state |
||
45 | * |
||
46 | * @param string $state The state to be added. |
||
47 | * @return void |
||
48 | */ |
||
49 | protected function _addAvailableState($state) { |
||
54 | |||
55 | /** |
||
56 | * Sets up all the methods that builds up the state machine. |
||
57 | * StateMachine->is<State> i.e. StateMachine->isParked() |
||
58 | * StateMachine->can<Transition> i.e. StateMachine->canShiftGear() |
||
59 | * StateMachine-><transition> i.e. StateMachine->shiftGear(); |
||
60 | * |
||
61 | * @param Model $model The model being used |
||
62 | * @param array $config Configuration for the Behavior |
||
63 | * @return void |
||
64 | */ |
||
65 | public function setup(Model $model, $config = array()) { |
||
90 | |||
91 | /** |
||
92 | * Adds a user defined callback |
||
93 | * {{{ |
||
94 | * $this->Vehicle->addMethod('myMethod', function() {}); |
||
95 | * $data = $this->Vehicle->myMethod(); |
||
96 | * }}} |
||
97 | * |
||
98 | * @param Model $model The model being acted on |
||
99 | * @param string $method The method na,e |
||
100 | * @param string $cb The callback to execute |
||
101 | * @throws InvalidArgumentException If the method already is registered |
||
102 | * @return void |
||
103 | */ |
||
104 | public function addMethod(Model $model, $method, $cb) { |
||
115 | |||
116 | /** |
||
117 | * Handles user defined method calls, which are implemented using closures. |
||
118 | * |
||
119 | * @param Model $model The model being acted on |
||
120 | * @param string $method The method name |
||
121 | * @return mixed The return value of the callback, or an array if the method doesn't exist |
||
122 | */ |
||
123 | public function handleMethodCall(Model $model, $method) { |
||
129 | |||
130 | /** |
||
131 | * Updates the model's state when a $model->save() call is performed |
||
132 | * |
||
133 | * @param Model $model The model being acted on |
||
134 | * @param bool $created Whether or not the model was created |
||
135 | * @param array $options Options passed to save |
||
136 | * @return bool |
||
137 | */ |
||
138 | public function afterSave(Model $model, $created, $options = array()) { |
||
145 | |||
146 | /** |
||
147 | * returns all transitions defined in model |
||
148 | * |
||
149 | * @param Model $model The model being acted on |
||
150 | * @return array array of transitions |
||
151 | * @author Frode Marton Meling |
||
152 | */ |
||
153 | public function getAllTransitions($model) { |
||
160 | |||
161 | /** |
||
162 | * Returns an array of all configured states |
||
163 | * |
||
164 | * @return array |
||
165 | */ |
||
166 | public function getAvailableStates() { |
||
169 | |||
170 | /** |
||
171 | * checks if $state or Array of states are valid ones |
||
172 | * |
||
173 | * @param string|array $state a string representation of state or a array of states |
||
174 | * @return bool |
||
175 | * @author Frode Marton Meling |
||
176 | */ |
||
177 | protected function _validState($state) { |
||
190 | |||
191 | /** |
||
192 | * Finds all records in a specific state. Supports additional conditions, but will overwrite conditions with state |
||
193 | * |
||
194 | * @param Model $model The model being acted on |
||
195 | * @param string $type find type (ref. CakeModel) |
||
196 | * @param array|string $state The state to find. this will be checked for validity. |
||
197 | * @param array $params Regular $params array for CakeModel->find |
||
198 | * @return array Returns datarray of $model records or false. Will return false if state is not set, or state is not configured in model |
||
199 | * @author Frode Marton Meling |
||
200 | */ |
||
201 | protected function _findByState(Model $model, $type, $state = null, $params = array()) { |
||
211 | |||
212 | /** |
||
213 | * This function will add all availble (runnable) transitions on a model and add it to the dataArray given to the function. |
||
214 | * |
||
215 | * @param Model $model The model being acted on |
||
216 | * @param array $modelRows The model dataArray. this is an array of Models returned from a model->find. |
||
217 | * @param string $role if specified, the function will limit the transitions based on a role |
||
218 | * @return array Returns datarray of $model with the available transitions inserted |
||
219 | * @author Frode Marton Meling |
||
220 | */ |
||
221 | protected function _addTransitionsToArray($model, $modelRows, $role) { |
||
239 | |||
240 | /** |
||
241 | * Finds all records in a specific state. Supports additional conditions, but will overwrite conditions with state |
||
242 | * |
||
243 | * @param Model $model The model being acted on |
||
244 | * @param array|string $state The state to find. this will be checked for validity. |
||
245 | * @param array $params Regular $params array for CakeModel->find |
||
246 | * @param bool $withTransitions Whether or not to add available transitions to records, in its current state |
||
247 | * @param string $role The rule executing the transition |
||
248 | * @return array Returns datarray of $model records or false. Will return false if state is not set, or state is not configured in model |
||
249 | * @author Frode Marton Meling |
||
250 | */ |
||
251 | public function findAllByState(Model $model, $state = null, $params = array(), $withTransitions = true, $role = null) { |
||
255 | |||
256 | /** |
||
257 | * Finds first record in a specific state. Supports additional conditions, but will overwrite conditions with state |
||
258 | * |
||
259 | * @param Model $model The model being acted on |
||
260 | * @param array|string $state The state to find. this will be checked for validity. |
||
261 | * @param array $params Regular $params array for CakeModel->find |
||
262 | * @param bool $withTransitions Whether or not to add available transitions to records, in its current state |
||
263 | * @param string $role The rule executing the transition |
||
264 | * @return array Returns datarray of $model records or false. Will return false if state is not set, or state is not configured in model |
||
265 | * @author Frode Marton Meling |
||
266 | */ |
||
267 | public function findFirstByState(Model $model, $state = null, $params = array(), $withTransitions = true, $role = null) { |
||
271 | |||
272 | /** |
||
273 | * Finds count of records in a specific state. Supports additional conditions, but will overwrite conditions with state |
||
274 | * |
||
275 | * @param Model $model The model being acted on |
||
276 | * @param array|string $state The state to find. this will be checked for validity. |
||
277 | * @param array $params Regular $params array for CakeModel->find |
||
278 | * @return array Returns datarray of $model records or false. Will return false if state is not set, or state is not configured in model |
||
279 | * @author Frode Marton Meling |
||
280 | */ |
||
281 | public function findCountByState(Model $model, $state = null, $params = array()) { |
||
284 | |||
285 | /** |
||
286 | * Allows moving from one state to another. |
||
287 | * {{{ |
||
288 | * $this->Model->transition('shift_gear'); |
||
289 | * // or |
||
290 | * $this->Model->shiftGear(); |
||
291 | * }}} |
||
292 | * |
||
293 | * @param Model $model The model being acted on |
||
294 | * @param string $transition The transition being initiated |
||
295 | * @param int $id table id field to find object |
||
296 | * @param string $role The rule executing the transition |
||
297 | * @param bool $validate whether or not validation being checked |
||
298 | * @return bool Returns true if the transition be executed, otherwise false |
||
299 | */ |
||
300 | public function transition(Model $model, $transition, $id = null, $role = null, $validate = true) { |
||
347 | |||
348 | /** |
||
349 | * Checks whether the state machine is in the given state |
||
350 | * |
||
351 | * @param Model $model The model being acted on |
||
352 | * @param string $state The state being checked |
||
353 | * @param int $id The id of the item to check |
||
354 | * @return bool whether or not the state machine is in the given state |
||
355 | * @throws BadMethodCallException when method does not exists |
||
356 | */ |
||
357 | public function is(Model $model, $state, $id = null) { |
||
367 | |||
368 | /** |
||
369 | * Checks whether or not the machine is able to perform transition, in its current state |
||
370 | * |
||
371 | * @param Model $model The model being acted on |
||
372 | * @param string $transition The transition being checked |
||
373 | * @param int $id The id of the item to check |
||
374 | * @param string $role The role which should execute the transition |
||
375 | * @return bool whether or not the machine can perform the transition |
||
376 | * @throws BadMethodCallException when method does not exists |
||
377 | */ |
||
378 | public function can(Model $model, $transition, $id = null, $role = null) { |
||
393 | |||
394 | /** |
||
395 | * Registers a callback function to be called when the machine leaves one state. |
||
396 | * The callback is fired either before or after the given transition. |
||
397 | * |
||
398 | * @param Model $model The model being acted on |
||
399 | * @param string $transition The transition to listen to |
||
400 | * @param string $triggerType Either before or after |
||
401 | * @param string $cb The callback function that will be called |
||
402 | * @param bool $bubble Whether or not to bubble other listeners |
||
403 | * @return void |
||
404 | */ |
||
405 | public function on(Model $model, $transition, $triggerType, $cb, $bubble = true) { |
||
411 | |||
412 | /** |
||
413 | * Registers a callback that will be called when the state machine enters the given |
||
414 | * state. |
||
415 | * |
||
416 | * @param Model $model The model being acted on |
||
417 | * @param string $state The state which the machine should enter |
||
418 | * @param string $cb The callback function that will be called |
||
419 | * @return void |
||
420 | */ |
||
421 | public function when(Model $model, $state, $cb) { |
||
424 | |||
425 | /** |
||
426 | * Returns the states the machine would be in, after the given transition |
||
427 | * |
||
428 | * @param Model $model The model being acted on |
||
429 | * @param string $transition The transition name |
||
430 | * @return mixed False if the transition doesnt yield any states, or an array of states |
||
431 | */ |
||
432 | public function getStates(Model $model, $transition) { |
||
452 | |||
453 | /** |
||
454 | * Returns the current state of the machine |
||
455 | * |
||
456 | * @param Model $model The model being acted on |
||
457 | * @param int $id The id of the item to check |
||
458 | * @return string The current state of the machine |
||
459 | */ |
||
460 | public function getCurrentState(Model $model, $id = null) { |
||
470 | |||
471 | /** |
||
472 | * Returns the previous state of the machine |
||
473 | * |
||
474 | * @param Model $model The model being acted on |
||
475 | * @param int $id The id of the item to check |
||
476 | * @return string The previous state of the machine |
||
477 | */ |
||
478 | View Code Duplication | public function getPreviousState(Model $model, $id = null) { |
|
488 | |||
489 | /** |
||
490 | * Returns the last transition ran |
||
491 | * |
||
492 | * @param Model $model The model being acted on |
||
493 | * @param int $id The id of the item to check |
||
494 | * @return string The transition last ran of the machine |
||
495 | */ |
||
496 | View Code Duplication | public function getLastTransition(Model $model, $id = null) { |
|
506 | |||
507 | /** |
||
508 | * Returns the role that ran the last transition |
||
509 | * |
||
510 | * @param Model $model The model being acted on |
||
511 | * @param int $id The id of the item to check |
||
512 | * @return string The role that last ran a transition of the machine |
||
513 | */ |
||
514 | View Code Duplication | public function getLastRole(Model $model, $id = null) { |
|
524 | |||
525 | /** |
||
526 | * Simple method to return contents for a GV file, that |
||
527 | * can be made into graphics by: |
||
528 | * {{{ |
||
529 | * dot -Tpng -ofsm.png fsm.gv |
||
530 | * }}} |
||
531 | * Assuming that the contents are written to the file fsm.gv |
||
532 | * |
||
533 | * @param Model $model The model being acted on |
||
534 | * @return string The contents of the graphviz file |
||
535 | */ |
||
536 | public function toDot(Model $model) { |
||
553 | |||
554 | /** |
||
555 | * This method prepares an array for each transition in the statemachine making it easier to iterate throug the machine for |
||
556 | * output to various formats |
||
557 | * |
||
558 | * @param Model $model The model being acted on |
||
559 | * @param array $roles The role(s) executing the transition change. with an options array. |
||
560 | * 'role' => array('color' => color of the arrows) |
||
561 | * In the future many more Graphviz options can be added |
||
562 | * @return array returns an array of all transitions |
||
563 | * @author Frode Marton Meling <[email protected]> |
||
564 | */ |
||
565 | public function prepareForDotWithRoles(Model $model, $roles) { |
||
593 | |||
594 | /** |
||
595 | * Method to return contents for a GV file based on array of roles. That means you can send |
||
596 | * an array of roles (with options) and this method will calculate the presentation that |
||
597 | * can be made into graphics by: |
||
598 | * {{{ |
||
599 | * dot -Tpng -ofsm.png fsm.gv |
||
600 | * }}} |
||
601 | * Assuming that the contents are written to the file fsm.gv |
||
602 | * |
||
603 | * @param Model $model The model being acted on |
||
604 | * @param array $roles The role(s) executing the transition change. with an options array. |
||
605 | * 'role' => array('color' => color of the arrows) |
||
606 | * In the future many more Graphviz options can be added |
||
607 | * @param array $dotOptions Options for nodes |
||
608 | * 'color' => 'color of all nodes' |
||
609 | * 'activeColor' => 'the color you want the active node to have' |
||
610 | * @return string The contents of the graphviz file |
||
611 | * @author Frode Marton Meling <[email protected]> |
||
612 | */ |
||
613 | public function createDotFileForRoles(Model $model, $roles, $dotOptions) { |
||
634 | |||
635 | /** |
||
636 | * This helperfunction fetches out all roles from an array of roles with options. Note that this is a ('role' => $options) array |
||
637 | * I did not find a php method for this, so made it myself |
||
638 | * |
||
639 | * @param Model $model The model being acted on |
||
640 | * @param Array $roles This is just an array of roles like array('role1', 'role2'...) |
||
641 | * @return Array Returns an array of roles like array('role1', 'role2'...) |
||
642 | * @author Frode Marton Meling <[email protected]> |
||
643 | * @todo Add separate tests @codingStandardsIgnoreLine |
||
644 | */ |
||
645 | public function getAllRoles(Model $model, $roles) { |
||
652 | |||
653 | /** |
||
654 | * This function is used to add transitions to Array. This tests for conditions and makes sure duplicates are not added. |
||
655 | * |
||
656 | * @param Model $model The model being acted on |
||
657 | * @param array $data An array of a transition to be added |
||
658 | * @param array $prepareArray The current array to populate |
||
659 | * @return mixed |
||
660 | * @author Frode Marton Meling <[email protected]> |
||
661 | * @todo Move this to protected, Needs a reimplementation of the functiun in test to make it public for testing @codingStandardsIgnoreLine |
||
662 | */ |
||
663 | public function addToPrepareArray(Model $model, $data, $prepareArray) { |
||
693 | |||
694 | /** |
||
695 | * This helperfunction checks if all roles in an array (roles) is present in $allArrays. Note that this is a ('role' => $options) array |
||
696 | * I did not find a php method for this, so made it myself |
||
697 | * |
||
698 | * @param Array $roles This is just an array of roles like array('role1', 'role2'...) |
||
699 | * @param Array $allRoles This is the array to test on. This is a multidimentional array like array('role1' => array('of' => 'options'), 'role2' => array('of' => 'options') ) |
||
700 | * @return bool Returns true if all roles are present, otherwise false |
||
701 | * @author Frode Marton Meling <[email protected]> |
||
702 | * @todo Add separate tests @codingStandardsIgnoreLine |
||
703 | */ |
||
704 | protected function _containsAllRoles($roles, $allRoles) { |
||
712 | |||
713 | /** |
||
714 | * This helperfunction checks if any of the roles in an array (roles) is present in $allArrays. Note that this is a ('role' => $options) array |
||
715 | * I did not find a php method for this, so made it myself |
||
716 | * |
||
717 | * @param Array $roles This is just an array of roles like array('role1', 'role2'...) |
||
718 | * @param Array $allRoles This is the array to test on. This is a multidimentional array like array('role1' => array('of' => 'options'), 'role2' => array('of' => 'options') ) |
||
719 | * @return bool Returns true if just one of the roles are present, otherwise false |
||
720 | * @author Frode Marton Meling <[email protected]> |
||
721 | * @todo Add separate tests @codingStandardsIgnoreLine |
||
722 | */ |
||
723 | protected function _containsAnyRoles($roles, $allRoles) { |
||
732 | |||
733 | /** |
||
734 | * This helperfunction adds a role to an array. It checks for duplicates and only adds if it is not already in array |
||
735 | * If also checks that the resultArray is valid and that there are roles there to begin with |
||
736 | * |
||
737 | * @param Array $roles This is just an array of roles like array('role1', 'role2'...) |
||
738 | * @param Array &$resultArray This function writes to this parameter by reference |
||
739 | * @return bool Returns true if added, otherwise false |
||
740 | * @author Frode Marton Meling <[email protected]> |
||
741 | * @todo Add separate tests @codingStandardsIgnoreLine |
||
742 | */ |
||
743 | protected function _addRoles($roles, &$resultArray) { |
||
753 | |||
754 | /** |
||
755 | * This helperfunction checks if state and transition is present in the array |
||
756 | * |
||
757 | * @param array $data The array to check |
||
758 | * @return bool true if array is valid, otherwise false |
||
759 | * @author Frode Marton Meling <[email protected]> |
||
760 | * @todo Add separate tests @codingStandardsIgnoreLine |
||
761 | */ |
||
762 | protected function _stateAndTransitionExist($data) { |
||
768 | |||
769 | /** |
||
770 | * This helperfunction checks if state, transition and depends exist in array |
||
771 | * |
||
772 | * @param array $data The array to check |
||
773 | * @return bool True if state, transition and depends exist in array, otherwise false |
||
774 | * @author Frode Marton Meling <[email protected]> |
||
775 | * @todo Add separate tests @codingStandardsIgnoreLine |
||
776 | */ |
||
777 | protected function _stateTransitionAndDependsExist($data) { |
||
783 | |||
784 | /** |
||
785 | * This helperfunction checks if state and transition is present in prepareArray. this is used to prevent adding duplicates |
||
786 | * |
||
787 | * @param array $data The array for testing |
||
788 | * @param array $prepareArray The array to check against |
||
789 | * @return bool index in array if state and transition is present in prepareArray, otherwise false |
||
790 | * @author Frode Marton Meling <[email protected]> |
||
791 | * @todo Add separate tests @codingStandardsIgnoreLine |
||
792 | */ |
||
793 | protected function _stateAndTransitionInArray($data, $prepareArray) { |
||
801 | |||
802 | /** |
||
803 | * This helperfunction checks if state, transition and depends is present in prepareArray. this is used to prevent adding duplicates |
||
804 | * |
||
805 | * @param array $data The array for testing |
||
806 | * @param array $prepareArray The array to check against |
||
807 | * @return bool the index in array if state, transition and depends is present in prepareArray, otherwise false |
||
808 | * @author Frode Marton Meling <[email protected]> |
||
809 | * @todo Add separate tests @codingStandardsIgnoreLine |
||
810 | */ |
||
811 | protected function _stateTransitionAndDependsInArray($data, $prepareArray) { |
||
822 | |||
823 | /** |
||
824 | * Checks whether or not the given role may perform the transition change. |
||
825 | * The callback in 'depends' must be a valid model method. |
||
826 | * |
||
827 | * @param Model $model The model being acted on |
||
828 | * @param string $role The role executing the transition change |
||
829 | * @param string $transition The transition |
||
830 | * @throws InvalidArgumentException if the transition require it be executed by a rule, and none is given |
||
831 | * @return bool Whether or not the role may perform the action |
||
832 | */ |
||
833 | protected function _checkRoleAgainstRule(Model $model, $role, $transition) { |
||
862 | |||
863 | /** |
||
864 | * Calls transition listeners before or after a particular transition. |
||
865 | * Special model methods are also called, if they exist: |
||
866 | * - onBeforeTransition |
||
867 | * - onAfterTransition |
||
868 | * - onBefore<Transition> i.e. onBeforePark() |
||
869 | * - onAfter<Transition> i.e. onAfterPark() |
||
870 | * |
||
871 | * @param Model $model The model being acted on |
||
872 | * @param string $transition The transition name |
||
873 | * @param string $triggerType Either before or after |
||
874 | * @return void |
||
875 | */ |
||
876 | protected function _callTransitionListeners(Model $model, $transition, $triggerType = 'after') { |
||
907 | |||
908 | /** |
||
909 | * Deformalizes a method name, removing 'can' and 'is' as well as underscoring |
||
910 | * the remaining text. |
||
911 | * |
||
912 | * @param string $name The model name |
||
913 | * @return string The deformalized method name |
||
914 | */ |
||
915 | protected function _deFormalizeMethodName($name) { |
||
918 | |||
919 | /** |
||
920 | * Checks whether or not a user-defined method exists in the Behavior |
||
921 | * |
||
922 | * @param Model $model The model being acted on |
||
923 | * @param string $method The method's name |
||
924 | * @return bool True if the method exists, false otherwise |
||
925 | */ |
||
926 | protected function _hasMethod(Model $model, $method) { |
||
929 | } |
||
930 |
The PSR-1: Basic Coding Standard recommends that a file should either introduce new symbols, that is classes, functions, constants or similar, or have side effects. Side effects are anything that executes logic, like for example printing output, changing ini settings or writing to a file.
The idea behind this recommendation is that merely auto-loading a class should not change the state of an application. It also promotes a cleaner style of programming and makes your code less prone to errors, because the logic is not spread out all over the place.
To learn more about the PSR-1, please see the PHP-FIG site on the PSR-1.