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 ScoreDisplay 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 ScoreDisplay, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 11 | class ScoreDisplay |
||
| 12 | { |
||
| 13 | private $coloring_enabled; |
||
| 14 | private $color_split_value; |
||
| 15 | private $custom_enabled; |
||
| 16 | private $upperlimit_included; |
||
| 17 | private $custom_display; |
||
| 18 | private $custom_display_conv; |
||
| 19 | |||
| 20 | /** |
||
| 21 | * Protected constructor - call instance() to instantiate |
||
| 22 | */ |
||
| 23 | public function __construct($category_id = 0) |
||
| 24 | { |
||
| 25 | if (!empty($category_id)) { |
||
| 26 | $this->category_id = $category_id; |
||
| 27 | } |
||
| 28 | |||
| 29 | // Loading portal settings + using standard functions. |
||
| 30 | |||
| 31 | $value = api_get_setting('gradebook_score_display_coloring'); |
||
| 32 | $value = $value['my_display_coloring']; |
||
| 33 | |||
| 34 | // Setting coloring. |
||
| 35 | $this->coloring_enabled = $value == 'true' ? true : false; |
||
| 36 | |||
| 37 | if ($this->coloring_enabled) { |
||
| 38 | $value = api_get_setting('gradebook_score_display_colorsplit'); |
||
| 39 | if (isset($value)) { |
||
| 40 | $this->color_split_value = $value; |
||
| 41 | } |
||
| 42 | } |
||
| 43 | |||
| 44 | //Setting custom enabled |
||
| 45 | $value = api_get_setting('gradebook_score_display_custom'); |
||
| 46 | $value = $value['my_display_custom']; |
||
| 47 | $this->custom_enabled = $value == 'true' ? true : false; |
||
| 48 | |||
| 49 | if ($this->custom_enabled) { |
||
| 50 | $params = array('category = ?' => array('Gradebook')); |
||
| 51 | $displays = api_get_settings_params($params); |
||
| 52 | $portal_displays = array(); |
||
| 53 | if (!empty($displays)) { |
||
| 54 | foreach ($displays as $display) { |
||
| 55 | $data = explode('::', $display['selected_value']); |
||
| 56 | if (empty($data[1])) { |
||
| 57 | $data[1] = ""; |
||
| 58 | } |
||
| 59 | $portal_displays[$data[0]] = array('score' => $data[0], 'display' => $data[1]); |
||
| 60 | } |
||
| 61 | sort($portal_displays); |
||
| 62 | } |
||
| 63 | $this->custom_display = $portal_displays; |
||
| 64 | if (count($this->custom_display)>0) { |
||
| 65 | $value = api_get_setting('gradebook_score_display_upperlimit'); |
||
| 66 | $value = $value['my_display_upperlimit']; |
||
| 67 | $this->upperlimit_included = $value == 'true' ? true : false; |
||
| 68 | $this->custom_display_conv = $this->convert_displays($this->custom_display); |
||
| 69 | } |
||
| 70 | } |
||
| 71 | |||
| 72 | //If teachers can override the portal parameters |
||
| 73 | |||
| 74 | if (api_get_setting('teachers_can_change_score_settings') == 'true') { |
||
| 75 | //Load course settings |
||
| 76 | if ($this->custom_enabled) { |
||
| 77 | $this->custom_display = $this->get_custom_displays(); |
||
| 78 | if (count($this->custom_display)> 0) { |
||
| 79 | $this->custom_display_conv = $this->convert_displays($this->custom_display); |
||
| 80 | } |
||
| 81 | } |
||
| 82 | |||
| 83 | if ($this->coloring_enabled) { |
||
| 84 | $this->color_split_value = $this->get_score_color_percent(); |
||
| 85 | } |
||
| 86 | } |
||
| 87 | } |
||
| 88 | |||
| 89 | /** |
||
| 90 | * Get the instance of this class |
||
| 91 | * @param int $category_id |
||
| 92 | */ |
||
| 93 | public static function instance($category_id = 0) |
||
| 94 | { |
||
| 95 | static $instance; |
||
| 96 | if (!isset ($instance)) { |
||
| 97 | $instance = new ScoreDisplay($category_id); |
||
| 98 | } |
||
| 99 | |||
| 100 | return $instance; |
||
| 101 | } |
||
| 102 | |||
| 103 | /** |
||
| 104 | * Compare the custom display of 2 scores, can be useful in sorting |
||
| 105 | */ |
||
| 106 | public static function compare_scores_by_custom_display ($score1, $score2) |
||
| 107 | { |
||
| 108 | if (!isset($score1)) { |
||
| 109 | return (isset($score2) ? 1 : 0); |
||
| 110 | } elseif (!isset($score2)) { |
||
| 111 | return -1; |
||
| 112 | } else { |
||
| 113 | $scoredisplay = ScoreDisplay :: instance(); |
||
| 114 | $custom1 = $scoredisplay->display_custom($score1); |
||
| 115 | $custom2 = $scoredisplay->display_custom($score2); |
||
| 116 | View Code Duplication | if ($custom1 == $custom2) { |
|
| 117 | return 0; |
||
| 118 | } else { |
||
| 119 | return (($score1[0]/$score1[1]) < ($score2[0]/$score2[1]) ? -1 : 1); |
||
| 120 | } |
||
| 121 | } |
||
| 122 | } |
||
| 123 | |||
| 124 | |||
| 125 | |||
| 126 | /** |
||
| 127 | * Is coloring enabled ? |
||
| 128 | */ |
||
| 129 | public function is_coloring_enabled() |
||
| 130 | { |
||
| 131 | return $this->coloring_enabled; |
||
| 132 | } |
||
| 133 | |||
| 134 | /** |
||
| 135 | * Is custom score display enabled ? |
||
| 136 | */ |
||
| 137 | public function is_custom() |
||
| 138 | { |
||
| 139 | return $this->custom_enabled; |
||
| 140 | } |
||
| 141 | |||
| 142 | /** |
||
| 143 | * Is upperlimit included ? |
||
| 144 | */ |
||
| 145 | public function is_upperlimit_included() |
||
| 146 | { |
||
| 147 | return $this->upperlimit_included; |
||
| 148 | } |
||
| 149 | |||
| 150 | /** |
||
| 151 | * If custom score display is enabled, this will return the current settings. |
||
| 152 | * See also update_custom_score_display_settings |
||
| 153 | * @return array current settings (or null if feature not enabled) |
||
| 154 | */ |
||
| 155 | public function get_custom_score_display_settings() |
||
| 156 | { |
||
| 157 | return $this->custom_display; |
||
| 158 | } |
||
| 159 | |||
| 160 | /** |
||
| 161 | * If coloring is enabled, scores below this value will be displayed in red. |
||
| 162 | * @return int color split value, in percent (or null if feature not enabled) |
||
| 163 | */ |
||
| 164 | public function get_color_split_value() |
||
| 168 | |||
| 169 | /** |
||
| 170 | * Get current gradebook category id |
||
| 171 | * @return int Category id |
||
| 172 | */ |
||
| 173 | private function get_current_gradebook_category_id() |
||
| 174 | { |
||
| 175 | $tbl_gradebook_category = Database :: get_main_table(TABLE_MAIN_GRADEBOOK_CATEGORY); |
||
| 196 | |||
| 197 | /** |
||
| 198 | * Update custom score display settings |
||
| 199 | * @param array $displays 2-dimensional array - every sub array must have keys (score, display) |
||
| 200 | * @param int score color percent (optional) |
||
| 201 | * @param int gradebook category id (optional) |
||
| 202 | */ |
||
| 203 | public function update_custom_score_display_settings($displays, $scorecolpercent = 0, $category_id = null) |
||
| 233 | |||
| 234 | /** |
||
| 235 | * @param int $category_id |
||
| 236 | * @return bool |
||
| 237 | */ |
||
| 238 | public function insert_defaults($category_id) |
||
| 265 | |||
| 266 | /** |
||
| 267 | * @return int|null|string |
||
| 268 | */ |
||
| 269 | public function get_number_decimals() |
||
| 278 | |||
| 279 | /** |
||
| 280 | * Formats a number depending of the number of decimals |
||
| 281 | * |
||
| 282 | * @param float $score |
||
| 283 | * @return float the score formatted |
||
| 284 | */ |
||
| 285 | public function format_score($score) |
||
| 289 | |||
| 290 | /** |
||
| 291 | * Display a score according to the current settings |
||
| 292 | * @param array $score data structure, as returned by the calc_score functions |
||
| 293 | * @param int $type one of the following constants: |
||
| 294 | * SCORE_DIV, SCORE_PERCENT, SCORE_DIV_PERCENT, SCORE_AVERAGE |
||
| 295 | * (ignored for student's view if custom score display is enabled) |
||
| 296 | * @param int $what one of the following constants: |
||
| 297 | * SCORE_BOTH, SCORE_ONLY_DEFAULT, SCORE_ONLY_CUSTOM (default: SCORE_BOTH) |
||
| 298 | * (only taken into account if custom score display is enabled and for course/platform admin) |
||
| 299 | * |
||
| 300 | * @return string |
||
| 301 | */ |
||
| 302 | public function display_score( |
||
| 338 | |||
| 339 | /** |
||
| 340 | * @param $score |
||
| 341 | * @param $type |
||
| 342 | * @return float|string |
||
| 343 | */ |
||
| 344 | private function display_default($score, $type) |
||
| 394 | |||
| 395 | /** |
||
| 396 | * @param array $score |
||
| 397 | * @return float|string |
||
| 398 | */ |
||
| 399 | private function display_simple_score($score) |
||
| 406 | |||
| 407 | /** |
||
| 408 | * Returns "1" for array("100", "100"); |
||
| 409 | * @param array $score |
||
| 410 | * @return float |
||
| 411 | */ |
||
| 412 | private function display_as_decimal($score) |
||
| 418 | |||
| 419 | /** |
||
| 420 | * Returns "100 %" for array("100", "100"); |
||
| 421 | */ |
||
| 422 | private function display_as_percent($score) |
||
| 428 | |||
| 429 | /** |
||
| 430 | * Returns 10.00 / 10.00 for array("100", "100"); |
||
| 431 | * @param array $score |
||
| 432 | * |
||
| 433 | * @return string |
||
| 434 | */ |
||
| 435 | private function display_as_div($score) |
||
| 445 | |||
| 446 | /** |
||
| 447 | * Depends on the teacher's configuration of thresholds. i.e. [0 50] "Bad", [50:100] "Good" |
||
| 448 | * @param array $score |
||
| 449 | */ |
||
| 450 | private function display_custom($score) |
||
| 470 | |||
| 471 | /** |
||
| 472 | * Get score color percent by category |
||
| 473 | * @param int Gradebook category id |
||
| 474 | * @return int Score |
||
| 475 | */ |
||
| 476 | private function get_score_color_percent($category_id = null) |
||
| 497 | |||
| 498 | /** |
||
| 499 | * Get current custom score display settings |
||
| 500 | * @param int Gradebook category id |
||
| 501 | * @return array 2-dimensional array every element contains 3 subelements (id, score, display) |
||
| 502 | */ |
||
| 503 | private function get_custom_displays($category_id = null) |
||
| 518 | |||
| 519 | /** |
||
| 520 | * Convert display settings to internally used values |
||
| 521 | */ |
||
| 522 | private function convert_displays($custom_display) |
||
| 556 | |||
| 557 | /** |
||
| 558 | * @param array $item1 |
||
| 559 | * @param array $item2 |
||
| 560 | * @return int |
||
| 561 | */ |
||
| 562 | View Code Duplication | private function sort_display($item1, $item2) |
|
| 571 | } |
||
| 572 |