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 ExecutionContainer 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 ExecutionContainer, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
51 | class ExecutionContainer extends AttributeHolder |
||
52 | { |
||
53 | /** |
||
54 | * @var Context The context instance. |
||
55 | */ |
||
56 | protected $context = null; |
||
57 | |||
58 | /** |
||
59 | * @var string The context name. |
||
60 | */ |
||
61 | protected $contextName = null; |
||
62 | |||
63 | /** |
||
64 | * @var string The output type name. |
||
65 | */ |
||
66 | protected $outputTypeName = null; |
||
67 | |||
68 | /** |
||
69 | * @var FilterChain The container's filter chain. |
||
70 | */ |
||
71 | protected $filterChain = null; |
||
72 | |||
73 | /** |
||
74 | * @var ValidationManager The validation manager instance. |
||
75 | */ |
||
76 | protected $validationManager = null; |
||
77 | |||
78 | /** |
||
79 | * @var string The request method for this container. |
||
80 | */ |
||
81 | protected $requestMethod = null; |
||
82 | |||
83 | /** |
||
84 | * @var RequestDataHolder A request data holder with request info. |
||
85 | */ |
||
86 | protected $requestData = null; // TODO: check if this can actually be protected |
||
87 | // or whether it should be private (would break controller tests though) |
||
88 | |||
89 | /** |
||
90 | * @var RequestDataHolder A pointer to the global request data. |
||
91 | */ |
||
92 | private $globalRequestData = null; |
||
93 | |||
94 | /** |
||
95 | * @var RequestDataHolder A request data holder with arguments. |
||
96 | */ |
||
97 | protected $arguments = null; |
||
98 | |||
99 | /** |
||
100 | * @var Response A response instance holding the Controller's output. |
||
101 | */ |
||
102 | protected $response = null; |
||
103 | |||
104 | /** |
||
105 | * @var OutputType The output type for this container. |
||
106 | */ |
||
107 | protected $outputType = null; |
||
108 | |||
109 | /** |
||
110 | * @var float The microtime at which this container was initialized. |
||
111 | */ |
||
112 | protected $microtime = null; |
||
113 | |||
114 | /** |
||
115 | * @var Controller The Controller instance that belongs to this container. |
||
116 | */ |
||
117 | protected $controllerInstance = null; |
||
118 | |||
119 | /** |
||
120 | * @var ViewException The View instance that belongs to this container. |
||
121 | */ |
||
122 | protected $viewInstance = null; |
||
123 | |||
124 | /** |
||
125 | * @var string The name of the Controller's Module. |
||
126 | */ |
||
127 | protected $moduleName = null; |
||
128 | |||
129 | /** |
||
130 | * @var string The name of the Controller. |
||
131 | */ |
||
132 | protected $controllerName = null; |
||
133 | |||
134 | /** |
||
135 | * @var string Name of the module of the View returned by the Controller. |
||
136 | */ |
||
137 | protected $viewModuleName = null; |
||
138 | |||
139 | /** |
||
140 | * @var string The name of the View returned by the Controller. |
||
141 | */ |
||
142 | protected $viewName = null; |
||
143 | |||
144 | /** |
||
145 | * @var ExecutionContainer The next container to execute. |
||
146 | */ |
||
147 | protected $next = null; |
||
148 | |||
149 | /** |
||
150 | * Controller names may contain any valid PHP token, as well as dots and slashes |
||
151 | * (for sub-controllers). |
||
152 | */ |
||
153 | const SANE_CONTROLLER_NAME = '/[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff\/.]*/'; |
||
154 | |||
155 | /** |
||
156 | * View names may contain any valid PHP token, as well as dots and slashes |
||
157 | * (for sub-controllers). |
||
158 | */ |
||
159 | const SANE_VIEW_NAME = '/[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff\/.]*/'; |
||
160 | |||
161 | /** |
||
162 | * Only valid PHP tokens are allowed in module names. |
||
163 | */ |
||
164 | const SANE_MODULE_NAME = '/[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*/'; |
||
165 | |||
166 | /** |
||
167 | * Pre-serialization callback. |
||
168 | * |
||
169 | * Will set the name of the context instead of the instance, and the name of |
||
170 | * the output type instead of the instance. Both will be restored by __wakeup |
||
171 | * |
||
172 | * @author David Zülke <[email protected]> |
||
173 | * @since 0.11.0 |
||
174 | */ |
||
175 | public function __sleep() |
||
185 | |||
186 | /** |
||
187 | * Post-unserialization callback. |
||
188 | * |
||
189 | * Will restore the context and output type instances based on their names set |
||
190 | * by __sleep. |
||
191 | * |
||
192 | * @author David Zülke <[email protected]> |
||
193 | * @since 0.11.0 |
||
194 | */ |
||
195 | public function __wakeup() |
||
210 | |||
211 | /** |
||
212 | * Initialize the container. This will create a response instance. |
||
213 | * |
||
214 | * @param Context $context The current Context instance. |
||
215 | * @param array $parameters An array of initialization parameters. |
||
216 | * |
||
217 | * @author David Zülke <[email protected]> |
||
218 | * @since 0.11.0 |
||
219 | */ |
||
220 | public function initialize(Context $context, array $parameters = array()) |
||
230 | |||
231 | /** |
||
232 | * Creates a new container instance with the same output type and request |
||
233 | * method as this one. |
||
234 | * |
||
235 | * @param string $moduleName The name of the module. |
||
236 | * @param string $controllerName The name of the controller. |
||
237 | * @param RequestDataHolder $arguments A RequestDataHolder with additional |
||
238 | * request arguments. |
||
239 | * @param string $outputType Optional name of an initial output type |
||
240 | * to set. |
||
241 | * @param string $requestMethod Optional name of the request method to |
||
242 | * be used in this container. |
||
243 | * |
||
244 | * @return ExecutionContainer A new execution container instance, |
||
245 | * fully initialized. |
||
246 | * |
||
247 | * @author David Zülke <[email protected]> |
||
248 | * @since 0.11.0 |
||
249 | */ |
||
250 | public function createExecutionContainer($moduleName = null, $controllerName = null, RequestDataHolder $arguments = null, $outputType = null, $requestMethod = null) |
||
266 | |||
267 | /** |
||
268 | * Start execution. |
||
269 | * |
||
270 | * This will create an instance of the controller and merge in request parameters. |
||
271 | * |
||
272 | * This method returns a response. It is not necessarily the same response as |
||
273 | * the one of this container, but instead the one that contains the actual |
||
274 | * content that should be used for output etc, since the container's own |
||
275 | * response might be empty or invalid due to a "next" container that has been |
||
276 | * set and executed. |
||
277 | * |
||
278 | * @return Response The "real" response. |
||
279 | * |
||
280 | * @author David Zülke <[email protected]> |
||
281 | * @since 0.11.0 |
||
282 | */ |
||
283 | public function execute() |
||
334 | |||
335 | /** |
||
336 | * Copies and merges the global request data. |
||
337 | * |
||
338 | * @author Felix Gilcher <[email protected]> |
||
339 | * @since 1.1.0 |
||
340 | */ |
||
341 | protected function initRequestData() |
||
360 | |||
361 | /** |
||
362 | * Create a system forward container |
||
363 | * |
||
364 | * Calling this method will set the attributes: |
||
365 | * - requested_module |
||
366 | * - requested_controller |
||
367 | * - (optional) exception |
||
368 | * in the appropriate namespace on the created container as well as the global |
||
369 | * request (for legacy reasons) |
||
370 | * |
||
371 | * |
||
372 | * @param string $type The type of forward to create (error_404, |
||
373 | * module_disabled, secure, login, unavailable). |
||
374 | * @param AgaviException $e Optional exception thrown by the Dispatcher |
||
375 | * while resolving the module/controller. |
||
376 | * |
||
377 | * @return ExecutionContainer The forward container. |
||
378 | * |
||
379 | * @author Felix Gilcher <[email protected]> |
||
380 | * @since 1.0.0 |
||
381 | */ |
||
382 | public function createSystemControllerForwardContainer($type, AgaviException $e = null) |
||
415 | |||
416 | /** |
||
417 | * Proceed to the "next" container by running it and returning its response, |
||
418 | * or return our response if there is no "next" container. |
||
419 | * |
||
420 | * @return Response The "real" response. |
||
421 | * |
||
422 | * @author David Zülke <[email protected]> |
||
423 | * @since 1.0.0 |
||
424 | */ |
||
425 | protected function proceed() |
||
433 | |||
434 | /** |
||
435 | * Get the Context. |
||
436 | * |
||
437 | * @return Context The Context. |
||
438 | * |
||
439 | * @author David Zülke <[email protected]> |
||
440 | * @since 0.11.0 |
||
441 | */ |
||
442 | final public function getContext() |
||
446 | |||
447 | /** |
||
448 | * Retrieve the ValidationManager |
||
449 | * |
||
450 | * @return ValidationManager The container's ValidationManager |
||
451 | * implementation instance. |
||
452 | * |
||
453 | * @author David Zülke <[email protected]> |
||
454 | * @since 0.11.0 |
||
455 | */ |
||
456 | public function getValidationManager() |
||
463 | |||
464 | /** |
||
465 | * Get the container's filter chain. |
||
466 | * |
||
467 | * @return FilterChain The container's filter chain. |
||
468 | * |
||
469 | * @author David Zülke <[email protected]> |
||
470 | * @since 1.1.0 |
||
471 | */ |
||
472 | View Code Duplication | public function getFilterChain() |
|
481 | |||
482 | /** |
||
483 | * Execute the Controller. |
||
484 | * |
||
485 | * @return mixed The processed View information returned by the Controller. |
||
486 | * |
||
487 | * @author David Zülke <[email protected]> |
||
488 | * @author Felix Gilcher <[email protected]> |
||
489 | * @since 1.0.0 |
||
490 | */ |
||
491 | public function runController() |
||
588 | |||
589 | /** |
||
590 | * Performs validation for this execution container. |
||
591 | * |
||
592 | * @return bool true if the data validated successfully, false otherwise. |
||
593 | * |
||
594 | * @author David Zülke <[email protected]> |
||
595 | * @author Felix Gilcher <[email protected]> |
||
596 | * @since 1.0.0 |
||
597 | */ |
||
598 | public function performValidation() |
||
625 | |||
626 | /** |
||
627 | * Register validators for this execution container. |
||
628 | * |
||
629 | * @author David Zülke <[email protected]> |
||
630 | * @author Felix Gilcher <[email protected]> |
||
631 | * @since 1.0.0 |
||
632 | */ |
||
633 | public function registerValidators() |
||
670 | |||
671 | /** |
||
672 | * Retrieve this container's request method name. |
||
673 | * |
||
674 | * @return string The request method name. |
||
675 | * |
||
676 | * @author David Zülke <[email protected]> |
||
677 | * @since 1.0.0 |
||
678 | */ |
||
679 | public function getRequestMethod() |
||
683 | |||
684 | /** |
||
685 | * Set this container's request method name. |
||
686 | * |
||
687 | * @param string $requestMethod The request method name. |
||
688 | * |
||
689 | * @author David Zülke <[email protected]> |
||
690 | * @since 1.0.0 |
||
691 | */ |
||
692 | public function setRequestMethod($requestMethod) |
||
696 | |||
697 | /** |
||
698 | * Retrieve this container's request data holder instance. |
||
699 | * |
||
700 | * @return RequestDataHolder The request data holder. |
||
701 | * |
||
702 | * @author David Zülke <[email protected]> |
||
703 | * @since 0.11.0 |
||
704 | */ |
||
705 | final public function getRequestData() |
||
709 | |||
710 | /** |
||
711 | * Set this container's global request data holder reference. |
||
712 | * |
||
713 | * @param RequestDataHolder $rd The request data holder. |
||
714 | * |
||
715 | * @author David Zülke <[email protected]> |
||
716 | * @since 0.11.0 |
||
717 | */ |
||
718 | final public function setRequestData(RequestDataHolder $rd) |
||
722 | |||
723 | /** |
||
724 | * Get this container's request data holder instance for additional arguments. |
||
725 | * |
||
726 | * @return RequestDataHolder The additional arguments. |
||
727 | * |
||
728 | * @author David Zülke <[email protected]> |
||
729 | * @since 0.11.0 |
||
730 | */ |
||
731 | public function getArguments() |
||
735 | |||
736 | /** |
||
737 | * Set this container's request data holder instance for additional arguments. |
||
738 | * |
||
739 | * @return RequestDataHolder The request data holder. |
||
740 | * |
||
741 | * @author David Zülke <[email protected]> |
||
742 | * @since 0.11.0 |
||
743 | */ |
||
744 | public function setArguments(RequestDataHolder $arguments) |
||
748 | |||
749 | /** |
||
750 | * Retrieve this container's response instance. |
||
751 | * |
||
752 | * @return Response The Response instance for this controller. |
||
753 | * |
||
754 | * @author David Zülke <[email protected]> |
||
755 | * @since 0.11.0 |
||
756 | */ |
||
757 | public function getResponse() |
||
761 | |||
762 | /** |
||
763 | * Set a new response. |
||
764 | * |
||
765 | * @param Response $response A new Response instance. |
||
766 | * |
||
767 | * @author David Zülke <[email protected]> |
||
768 | * @since 0.11.0 |
||
769 | */ |
||
770 | public function setResponse(Response $response) |
||
775 | |||
776 | /** |
||
777 | * Retrieve the output type of this container. |
||
778 | * |
||
779 | * @return OutputType The output type object. |
||
780 | * |
||
781 | * @author David Zülke <[email protected]> |
||
782 | * @since 0.11.0 |
||
783 | */ |
||
784 | public function getOutputType() |
||
788 | |||
789 | /** |
||
790 | * Set a different output type for this container. |
||
791 | * |
||
792 | * @param OutputType $outputType An output type object. |
||
793 | * |
||
794 | * @author David Zülke <[email protected]> |
||
795 | * @since 0.11.0 |
||
796 | */ |
||
797 | public function setOutputType(OutputType $outputType) |
||
804 | |||
805 | /** |
||
806 | * Retrieve this container's microtime. |
||
807 | * |
||
808 | * @return string A string representing the microtime this container was |
||
809 | * initialized. |
||
810 | * |
||
811 | * @author David Zülke <[email protected]> |
||
812 | * @since 0.11.0 |
||
813 | */ |
||
814 | public function getMicrotime() |
||
818 | |||
819 | /** |
||
820 | * Retrieve this container's controller instance. |
||
821 | * |
||
822 | * @return Controller An controller implementation instance. |
||
823 | * |
||
824 | * @author David Zülke <[email protected]> |
||
825 | * @since 0.11.0 |
||
826 | */ |
||
827 | public function getControllerInstance() |
||
843 | |||
844 | /** |
||
845 | * Retrieve this container's view instance. |
||
846 | * |
||
847 | * @return View A view implementation instance. |
||
848 | * |
||
849 | * @author Ross Lawley <[email protected]> |
||
850 | * @since 0.11.0 |
||
851 | */ |
||
852 | public function getViewInstance() |
||
863 | |||
864 | /** |
||
865 | * Set this container's view instance. |
||
866 | * |
||
867 | * @param View $viewInstance A view implementation instance. |
||
868 | * |
||
869 | * @author Ross Lawley <[email protected]> |
||
870 | * @since 0.11.0 |
||
871 | */ |
||
872 | public function setViewInstance($viewInstance) |
||
876 | |||
877 | /** |
||
878 | * Retrieve this container's module name. |
||
879 | * |
||
880 | * @return string A module name. |
||
881 | * |
||
882 | * @author David Zülke <[email protected]> |
||
883 | * @since 0.11.0 |
||
884 | */ |
||
885 | public function getModuleName() |
||
889 | |||
890 | /** |
||
891 | * Retrieve this container's controller name. |
||
892 | * |
||
893 | * @return string An controller name. |
||
894 | * |
||
895 | * @author David Zülke <[email protected]> |
||
896 | * @since 0.11.0 |
||
897 | */ |
||
898 | public function getControllerName() |
||
902 | |||
903 | /** |
||
904 | * Retrieve this container's view module name. This is the name of the module of |
||
905 | * the View returned by the Controller. |
||
906 | * |
||
907 | * @return string A view module name. |
||
908 | * |
||
909 | * @author David Zülke <[email protected]> |
||
910 | * @since 0.11.0 |
||
911 | */ |
||
912 | public function getViewModuleName() |
||
916 | |||
917 | /** |
||
918 | * Retrieve this container's view name. |
||
919 | * |
||
920 | * @return string A view name. |
||
921 | * |
||
922 | * @author David Zülke <[email protected]> |
||
923 | * @since 0.11.0 |
||
924 | */ |
||
925 | public function getViewName() |
||
929 | |||
930 | /** |
||
931 | * Set the module name for this container. |
||
932 | * |
||
933 | * @param string $moduleName A module name. |
||
934 | * |
||
935 | * @author David Zülke <[email protected]> |
||
936 | * @since 0.11.0 |
||
937 | */ |
||
938 | public function setModuleName($moduleName) |
||
948 | |||
949 | /** |
||
950 | * Set the controller name for this container. |
||
951 | * |
||
952 | * @param string $controllerName An controller name. |
||
953 | * |
||
954 | * @author David Zülke <[email protected]> |
||
955 | * @since 0.11.0 |
||
956 | */ |
||
957 | public function setControllerName($controllerName) |
||
968 | |||
969 | /** |
||
970 | * Set the view module name for this container. |
||
971 | * |
||
972 | * @param string $viewModuleName A view module name. |
||
973 | * |
||
974 | * @author David Zülke <[email protected]> |
||
975 | * @since 0.11.0 |
||
976 | */ |
||
977 | public function setViewModuleName($viewModuleName) |
||
987 | |||
988 | /** |
||
989 | * Set the module name for this container. |
||
990 | * |
||
991 | * @param string $viewName A view name. |
||
992 | * |
||
993 | * @author David Zülke <[email protected]> |
||
994 | * @since 0.11.0 |
||
995 | */ |
||
996 | public function setViewName($viewName) |
||
1007 | |||
1008 | /** |
||
1009 | * Check if a "next" container has been set. |
||
1010 | * |
||
1011 | * @return bool True, if a container for eventual execution has been set. |
||
1012 | * |
||
1013 | * @author David Zülke <[email protected]> |
||
1014 | * @since 0.11.0 |
||
1015 | */ |
||
1016 | public function hasNext() |
||
1020 | |||
1021 | /** |
||
1022 | * Get the "next" container. |
||
1023 | * |
||
1024 | * @return ExecutionContainer The "next" container, of null if unset. |
||
1025 | * |
||
1026 | * @author David Zülke <[email protected]> |
||
1027 | * @since 0.11.0 |
||
1028 | */ |
||
1029 | public function getNext() |
||
1033 | |||
1034 | /** |
||
1035 | * Set the container that should be executed once this one finished running. |
||
1036 | * |
||
1037 | * @param ExecutionContainer $container An execution container instance. |
||
1038 | * |
||
1039 | * @author David Zülke <[email protected]> |
||
1040 | * @since 0.11.0 |
||
1041 | */ |
||
1042 | public function setNext(ExecutionContainer $container) |
||
1046 | |||
1047 | /** |
||
1048 | * Remove a possibly set "next" container. |
||
1049 | * |
||
1050 | * @return ExecutionContainer The removed "next" container, or null |
||
1051 | * if none had been set. |
||
1052 | * |
||
1053 | * @author David Zülke <[email protected]> |
||
1054 | * @since 0.11.0 |
||
1055 | */ |
||
1056 | public function clearNext() |
||
1062 | } |
||
1063 |
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: