phpbb /
ideas
| Total Complexity | 41 |
| Total Lines | 266 |
| Duplicated Lines | 0 % |
| Changes | 8 | ||
| Bugs | 1 | Features | 0 |
Complex classes like idea_controller 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.
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 idea_controller, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 18 | class idea_controller extends base |
||
| 19 | { |
||
| 20 | /** @var array of idea data */ |
||
| 21 | protected $data; |
||
| 22 | |||
| 23 | /* @var \phpbb\ideas\factory\idea */ |
||
| 24 | protected $entity; |
||
| 25 | |||
| 26 | /** |
||
| 27 | * Controller for /idea/{idea_id} |
||
| 28 | * |
||
| 29 | * @param $idea_id int The ID of the requested idea, maybe? |
||
| 30 | * @throws http_exception |
||
| 31 | * @return JsonResponse|RedirectResponse |
||
| 32 | */ |
||
| 33 | public function idea($idea_id) |
||
| 34 | { |
||
| 35 | if (!$this->is_available()) |
||
| 36 | { |
||
| 37 | throw new http_exception(404, 'IDEAS_NOT_AVAILABLE'); |
||
| 38 | } |
||
| 39 | |||
| 40 | $this->data = $this->entity->get_idea($idea_id); |
||
|
|
|||
| 41 | if (!$this->data) |
||
| 42 | { |
||
| 43 | throw new http_exception(404, 'IDEA_NOT_FOUND'); |
||
| 44 | } |
||
| 45 | |||
| 46 | $mode = $this->request->variable('mode', ''); |
||
| 47 | if (!empty($mode) && $this->request->is_ajax()) |
||
| 48 | { |
||
| 49 | $result = $this->$mode(); |
||
| 50 | |||
| 51 | return new JsonResponse($result); |
||
| 52 | } |
||
| 53 | |||
| 54 | $params = array( |
||
| 55 | 'f' => $this->config['ideas_forum_id'], |
||
| 56 | 't' => $this->data['topic_id'] |
||
| 57 | ); |
||
| 58 | |||
| 59 | if ($unread = ($this->request->variable('view', '') === 'unread')) |
||
| 60 | { |
||
| 61 | $params = array_merge($params, array('view' => 'unread')); |
||
| 62 | } |
||
| 63 | |||
| 64 | $url = append_sid(generate_board_url() . "/viewtopic.$this->php_ext", $params, false) . ($unread ? '#unread' : ''); |
||
| 65 | |||
| 66 | return new RedirectResponse($url); |
||
| 67 | } |
||
| 68 | |||
| 69 | /** |
||
| 70 | * Delete action (deletes an idea via confirm dialog) |
||
| 71 | * |
||
| 72 | * @throws http_exception |
||
| 73 | * @return void |
||
| 74 | * @access public |
||
| 75 | */ |
||
| 76 | public function delete() |
||
| 108 | ) |
||
| 109 | ) |
||
| 110 | ); |
||
| 111 | } |
||
| 112 | } |
||
| 113 | |||
| 114 | /** |
||
| 115 | * Duplicate action (sets an idea's duplicate link) |
||
| 116 | * |
||
| 117 | * @return bool True if set, false if not |
||
| 118 | * @access public |
||
| 119 | */ |
||
| 120 | public function duplicate() |
||
| 121 | { |
||
| 122 | if ($this->is_mod() && check_link_hash($this->get_hash(), "duplicate_{$this->data['idea_id']}")) |
||
| 123 | { |
||
| 124 | $duplicate = $this->request->variable('duplicate', 0); |
||
| 125 | return $this->entity->set_duplicate($this->data['idea_id'], $duplicate); |
||
| 126 | } |
||
| 127 | |||
| 128 | return false; |
||
| 129 | } |
||
| 130 | |||
| 131 | /** |
||
| 132 | * Remove vote action (remove a user's vote from an idea) |
||
| 133 | * |
||
| 134 | * @return array|false|string Array of vote data, an error message, or false if failed |
||
| 135 | * @access public |
||
| 136 | */ |
||
| 137 | public function removevote() |
||
| 154 | } |
||
| 155 | |||
| 156 | /** |
||
| 157 | * RFC action (sets an idea's rfc link) |
||
| 158 | * |
||
| 159 | * @return bool True if set, false if not |
||
| 160 | * @access public |
||
| 161 | */ |
||
| 162 | public function rfc() |
||
| 163 | { |
||
| 164 | if (($this->is_own() || $this->is_mod()) && check_link_hash($this->get_hash(), "rfc_{$this->data['idea_id']}")) |
||
| 165 | { |
||
| 166 | $rfc = $this->request->variable('rfc', ''); |
||
| 167 | return $this->entity->set_rfc($this->data['idea_id'], $rfc); |
||
| 168 | } |
||
| 169 | |||
| 170 | return false; |
||
| 171 | } |
||
| 172 | |||
| 173 | /** |
||
| 174 | * Status action (sets an idea's status) |
||
| 175 | * |
||
| 176 | * @return bool True if set, false if not |
||
| 177 | * @access public |
||
| 178 | */ |
||
| 179 | public function status() |
||
| 180 | { |
||
| 181 | $status = $this->request->variable('status', 0); |
||
| 182 | |||
| 183 | if ($status && $this->is_mod() && check_link_hash($this->get_hash(), "status_{$this->data['idea_id']}")) |
||
| 184 | { |
||
| 185 | $this->entity->set_status($this->data['idea_id'], $status); |
||
| 186 | return true; |
||
| 187 | } |
||
| 188 | |||
| 189 | return false; |
||
| 190 | } |
||
| 191 | |||
| 192 | /** |
||
| 193 | * Ticket action (sets an idea's ticket link) |
||
| 194 | * |
||
| 195 | * @return bool True if set, false if not |
||
| 196 | * @access public |
||
| 197 | */ |
||
| 198 | public function ticket() |
||
| 199 | { |
||
| 200 | if (($this->is_own() || $this->is_mod()) && check_link_hash($this->get_hash(), "ticket_{$this->data['idea_id']}")) |
||
| 201 | { |
||
| 202 | $ticket = $this->request->variable('ticket', 0); |
||
| 203 | return $this->entity->set_ticket($this->data['idea_id'], $ticket); |
||
| 204 | } |
||
| 205 | |||
| 206 | return false; |
||
| 207 | } |
||
| 208 | |||
| 209 | /** |
||
| 210 | * Implemented action (sets an idea's implemented phpBB version) |
||
| 211 | * |
||
| 212 | * @return bool True if set, false if not |
||
| 213 | * @access public |
||
| 214 | */ |
||
| 215 | public function implemented() |
||
| 216 | { |
||
| 217 | if ($this->is_mod() && check_link_hash($this->get_hash(), "implemented_{$this->data['idea_id']}")) |
||
| 218 | { |
||
| 219 | $version = $this->request->variable('implemented', ''); |
||
| 220 | return $this->entity->set_implemented($this->data['idea_id'], $version); |
||
| 221 | } |
||
| 222 | |||
| 223 | return false; |
||
| 224 | } |
||
| 225 | |||
| 226 | /** |
||
| 227 | * Vote action (sets an idea's vote) |
||
| 228 | * |
||
| 229 | * @return array|false|string Array of vote data, an error message, or false if failed |
||
| 230 | * @access public |
||
| 231 | */ |
||
| 232 | public function vote() |
||
| 251 | } |
||
| 252 | |||
| 253 | /** |
||
| 254 | * Get a hash query parameter |
||
| 255 | * |
||
| 256 | * @return string The hash |
||
| 257 | * @access protected |
||
| 258 | */ |
||
| 259 | protected function get_hash() |
||
| 260 | { |
||
| 261 | return $this->request->variable('hash', ''); |
||
| 262 | } |
||
| 263 | |||
| 264 | /** |
||
| 265 | * Does the user have moderator privileges? |
||
| 266 | * |
||
| 267 | * @return bool |
||
| 268 | * @access protected |
||
| 269 | */ |
||
| 270 | protected function is_mod() |
||
| 271 | { |
||
| 272 | return $this->auth->acl_get('m_', (int) $this->config['ideas_forum_id']); |
||
| 273 | } |
||
| 274 | |||
| 275 | /** |
||
| 276 | * Is the user the author of the idea? |
||
| 277 | * |
||
| 278 | * @return bool |
||
| 279 | * @access protected |
||
| 280 | */ |
||
| 281 | protected function is_own() |
||
| 284 | } |
||
| 285 | } |
||
| 286 |
Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.
For example, imagine you have a variable
$accountIdthat can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to theidproperty of an instance of theAccountclass. This class holds a proper account, so the id value must no longer be false.Either this assignment is in error or a type check should be added for that assignment.