Complex classes like ShortListController 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 ShortListController, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
4 | class ShortListController extends Page_Controller |
||
5 | { |
||
6 | private static $allowed_actions = array( |
||
7 | 'renderList', |
||
8 | 'performAction' |
||
9 | ); |
||
10 | |||
11 | private static $url_handlers = array( |
||
12 | 'add' => 'performAction', |
||
13 | 'remove' => 'performAction', |
||
14 | '$URL!' => 'renderList', |
||
15 | ); |
||
16 | |||
17 | private static $extensions = array( |
||
18 | 'ShortListPaginationExtension' |
||
19 | ); |
||
20 | |||
21 | public function init() |
||
31 | |||
32 | /** |
||
33 | * When landing on the homepage, if there is a shortlist for the current |
||
34 | * user, redirect to the correct URL. Otherwise, 404. |
||
35 | * */ |
||
36 | public function index($request) |
||
59 | |||
60 | /** |
||
61 | * Get the absolute URL of this controller. |
||
62 | * */ |
||
63 | public function Link($action = null) |
||
74 | |||
75 | public function renderList($request) |
||
97 | |||
98 | public function performAction($request) |
||
130 | |||
131 | |||
132 | /** |
||
133 | * Get the number of items in the current short list. |
||
134 | * |
||
135 | * @param session The session to check & find a shortlist for. |
||
136 | * @return mixed false if no session exists - else the number of items in the shortlist. |
||
137 | * */ |
||
138 | public function shortListCount($session = false) |
||
152 | |||
153 | public static function getShortListSession() |
||
157 | |||
158 | /** |
||
159 | * Get the token to use to add/remove from shortlist. |
||
160 | * */ |
||
161 | public static function getSecurityToken() |
||
165 | |||
166 | /** |
||
167 | * Return a valid shortlist - or null. |
||
168 | * */ |
||
169 | private function getSessionShortList() |
||
176 | |||
177 | /** |
||
178 | * Return the json encoded count & url for the current session |
||
179 | * */ |
||
180 | private function renderAjax($session) { |
||
193 | |||
194 | /** |
||
195 | * Don't render the template! |
||
196 | * */ |
||
197 | private function dontRender($shortlist, $request) |
||
201 | |||
202 | /** |
||
203 | * Is this session valid? |
||
204 | * */ |
||
205 | private function isSessionValid($session) { |
||
208 | |||
209 | /** |
||
210 | * Don't perform an action. |
||
211 | * */ |
||
212 | private function dontPerformAction($request) |
||
217 | } |
||
218 |
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: