Complex classes like featured_member 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 featured_member, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 22 | class featured_member extends block |
||
| 23 | { |
||
| 24 | /** @var cache */ |
||
| 25 | protected $cache; |
||
| 26 | |||
| 27 | /** @var config */ |
||
| 28 | protected $config; |
||
| 29 | |||
| 30 | /** @var database */ |
||
| 31 | protected $db; |
||
| 32 | |||
| 33 | /** @var profile_fields */ |
||
| 34 | protected $profile_fields; |
||
| 35 | |||
| 36 | /** @var user */ |
||
| 37 | protected $user; |
||
| 38 | |||
| 39 | /** @var string */ |
||
| 40 | protected $phpbb_root_path; |
||
| 41 | |||
| 42 | /** @var string */ |
||
| 43 | protected $php_ext; |
||
| 44 | |||
| 45 | /** @var string */ |
||
| 46 | protected $blocks_table; |
||
| 47 | |||
| 48 | /** @var int */ |
||
| 49 | protected $cache_time; |
||
| 50 | |||
| 51 | /** @var array */ |
||
| 52 | private $settings; |
||
| 53 | |||
| 54 | /** @var array */ |
||
| 55 | private static $rotations = array( |
||
| 56 | 'hourly' => 'hour', |
||
| 57 | 'daily' => 'day', |
||
| 58 | 'weekly' => 'week', |
||
| 59 | 'monthly' => 'month' |
||
| 60 | ); |
||
| 61 | |||
| 62 | /** |
||
| 63 | * Constructor |
||
| 64 | * |
||
| 65 | * @param cache $cache Cache driver interface |
||
| 66 | * @param config $config Config object |
||
| 67 | * @param database $db Database connection |
||
| 68 | * @param profile_fields $profile_fields Profile fields manager object |
||
| 69 | * @param user $user User object |
||
| 70 | * @param string $phpbb_root_path Path to the phpbb includes directory. |
||
| 71 | * @param string $php_ext php file extension |
||
| 72 | * @param string $blocks_table Name of blocks database table |
||
| 73 | * @param int $cache_time |
||
| 74 | */ |
||
| 75 | public function __construct(cache $cache, config $config, database $db, profile_fields $profile_fields, user $user, $phpbb_root_path, $php_ext, $blocks_table, $cache_time = 3600) |
||
| 92 | |||
| 93 | /** |
||
| 94 | * {@inheritdoc} |
||
| 95 | */ |
||
| 96 | public function get_config(array $settings) |
||
| 114 | |||
| 115 | /** |
||
| 116 | * {@inheritdoc} |
||
| 117 | */ |
||
| 118 | public function display(array $bdata, $edit_mode = false) |
||
| 139 | |||
| 140 | /** |
||
| 141 | * @param bool $change_user |
||
| 142 | * @return array|false |
||
| 143 | */ |
||
| 144 | private function _get_user_data($change_user) |
||
| 167 | |||
| 168 | /** |
||
| 169 | * @param string $sql |
||
| 170 | * @param bool $change_user |
||
| 171 | */ |
||
| 172 | private function _query_user_by_featured(&$sql, $change_user) |
||
| 179 | |||
| 180 | /** |
||
| 181 | * @param string $sql |
||
| 182 | */ |
||
| 183 | private function _query_user_by_recent(&$sql) |
||
| 187 | |||
| 188 | /** |
||
| 189 | * @param string $sql |
||
| 190 | */ |
||
| 191 | private function _query_user_by_posts(&$sql) |
||
| 195 | |||
| 196 | /** |
||
| 197 | * @param string $query_type |
||
| 198 | * @param string $rotation |
||
| 199 | * @return int |
||
| 200 | */ |
||
| 201 | private function _get_cache_time($query_type, $rotation) |
||
| 205 | |||
| 206 | /** |
||
| 207 | * @return bool |
||
| 208 | */ |
||
| 209 | private function _change_user() |
||
| 221 | |||
| 222 | /** |
||
| 223 | * @param array $userlist |
||
| 224 | * @param $change_user |
||
| 225 | * @return int |
||
| 226 | */ |
||
| 227 | private function _get_featured_user(array $userlist, $change_user) |
||
| 240 | |||
| 241 | /** |
||
| 242 | * @param $current_user |
||
| 243 | * @param $userlist |
||
| 244 | * @return int |
||
| 245 | */ |
||
| 246 | private function _get_next_user($current_user, array $userlist) |
||
| 256 | |||
| 257 | /** |
||
| 258 | * @param $current_user |
||
| 259 | * @param $userlist |
||
| 260 | * @return int |
||
| 261 | */ |
||
| 262 | private function _get_next_key($current_user, array $userlist) |
||
| 269 | |||
| 270 | /** |
||
| 271 | */ |
||
| 272 | private function _explain_view() |
||
| 282 | |||
| 283 | /** |
||
| 284 | * @param array $bdata |
||
| 285 | * @return array |
||
| 286 | */ |
||
| 287 | private function _get_settings(array $bdata) |
||
| 295 | |||
| 296 | /** |
||
| 297 | * @param $bid |
||
| 298 | * @param bool $change_user |
||
| 299 | */ |
||
| 300 | private function _save_settings($bid, $change_user) |
||
| 313 | |||
| 314 | /** |
||
| 315 | * @param string $list |
||
| 316 | * @return array |
||
| 317 | */ |
||
| 318 | private function _get_userlist($list) |
||
| 323 | |||
| 324 | /** |
||
| 325 | * if we're selecting from a list and there is no result, we remove the culprit and update the list |
||
| 326 | * |
||
| 327 | * @param string $title |
||
| 328 | * @param array $bdata |
||
| 329 | * @param bool $edit_mode |
||
| 330 | * @return array |
||
| 331 | */ |
||
| 332 | private function _update_userlist($title, array $bdata, $edit_mode) |
||
| 361 | |||
| 362 | /** |
||
| 363 | * @param $user_id |
||
| 364 | * @return array|string |
||
| 365 | */ |
||
| 366 | private function _get_profile_fields($user_id) |
||
| 381 | |||
| 382 | /** |
||
| 383 | * @param int $user_id |
||
| 384 | */ |
||
| 385 | private function _show_profile_fields($user_id) |
||
| 399 | |||
| 400 | /** |
||
| 401 | * @param array $row |
||
| 402 | * @return array |
||
| 403 | */ |
||
| 404 | private function _display_user(array $row) |
||
| 431 | |||
| 432 | /** |
||
| 433 | * @param $user_posts |
||
| 434 | * @return int|mixed |
||
| 435 | */ |
||
| 436 | private function _calculate_percent_posts($user_posts) |
||
| 440 | |||
| 441 | /** |
||
| 442 | * @param int $last_visited |
||
| 443 | * @param string $date_format |
||
| 444 | * @return string |
||
| 445 | */ |
||
| 446 | private function _get_last_visit_date($last_visited, $date_format) |
||
| 450 | |||
| 451 | /** |
||
| 452 | * @return array |
||
| 453 | */ |
||
| 454 | private function _get_rotation_frequencies() |
||
| 464 | |||
| 465 | /** |
||
| 466 | * @return array |
||
| 467 | */ |
||
| 468 | private function _get_query_types() |
||
| 477 | |||
| 478 | /** |
||
| 479 | * @return array |
||
| 480 | */ |
||
| 481 | private function _get_cpf_fields() |
||
| 502 | } |
||
| 503 |
This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.
Consider making the comparison explicit by using
empty(..)or! empty(...)instead.