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 Package 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 Package, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 15 | class Package extends Model |
||
| 16 | { |
||
| 17 | /** |
||
| 18 | * @var string CRC32 temporary uniq id of the package |
||
| 19 | */ |
||
| 20 | private $_id; |
||
| 21 | |||
| 22 | /** @var Tariff */ |
||
| 23 | protected $_tariff; |
||
| 24 | |||
| 25 | /** @var Part[] */ |
||
| 26 | public $parts; |
||
| 27 | |||
| 28 | /** @var array */ |
||
| 29 | public $calculation; |
||
| 30 | |||
| 31 | /** |
||
| 32 | * @var |
||
| 33 | */ |
||
| 34 | protected $_resources; |
||
| 35 | |||
| 36 | public function init() |
||
| 40 | |||
| 41 | /** |
||
| 42 | * @param Tariff $tariff |
||
| 43 | */ |
||
| 44 | public function setTariff($tariff) |
||
| 48 | |||
| 49 | /** |
||
| 50 | * @throws InvalidConfigException |
||
| 51 | * TODO: implement and get rid of many magic functions bellow |
||
| 52 | */ |
||
| 53 | protected function initResources() |
||
| 64 | |||
| 65 | /** |
||
| 66 | * @param $resource |
||
| 67 | * @return string |
||
| 68 | * TODO: implement and get rid of many magic functions bellow |
||
| 69 | */ |
||
| 70 | protected function buildResourceClass($resource) |
||
| 78 | |||
| 79 | /** |
||
| 80 | * @return Tariff |
||
| 81 | */ |
||
| 82 | public function getTariff() |
||
| 86 | |||
| 87 | protected function getResourceTitle_cpu() |
||
| 91 | |||
| 92 | protected function getResourceValue_cpu() |
||
| 98 | |||
| 99 | protected function getResourceTitle_ram() |
||
| 103 | |||
| 104 | protected function getResourceValue_ram() |
||
| 110 | |||
| 111 | View Code Duplication | protected function getResourceOveruse_ram() |
|
| 118 | |||
| 119 | protected function getResourceTitle_hdd() |
||
| 123 | |||
| 124 | protected function getResourceValue_hdd() |
||
| 130 | |||
| 131 | View Code Duplication | protected function getResourceOveruse_hdd() |
|
| 139 | |||
| 140 | protected function getResourceTitle_ip() |
||
| 144 | |||
| 145 | protected function getResourceValue_ip() |
||
| 149 | |||
| 150 | View Code Duplication | protected function getResourceOveruse_ip() |
|
| 158 | |||
| 159 | protected function getResourceTitle_support_time() |
||
| 163 | |||
| 164 | protected function getResourceValue_support_time() |
||
| 179 | |||
| 180 | View Code Duplication | protected function getResourceOveruse_traffic() |
|
| 188 | |||
| 189 | protected function getResourceTitle_traffic() |
||
| 193 | |||
| 194 | protected function getResourceValue_traffic() |
||
| 199 | |||
| 200 | protected function getResourceValue_speed() |
||
| 204 | |||
| 205 | protected function getResourceTitle_speed() |
||
| 209 | |||
| 210 | protected function getResourceValue_panel() |
||
| 220 | |||
| 221 | protected function getResourceTitle_panel() |
||
| 225 | |||
| 226 | protected function getResourceValue_purpose() |
||
| 230 | |||
| 231 | protected function getResourceTitle_purpose() |
||
| 235 | |||
| 236 | |||
| 237 | /** |
||
| 238 | * @return float |
||
| 239 | */ |
||
| 240 | public function getPrice() |
||
| 244 | |||
| 245 | /** |
||
| 246 | * @return string |
||
| 247 | * @throws InvalidConfigException |
||
| 248 | */ |
||
| 249 | public function getDisplayPrice() |
||
| 255 | |||
| 256 | /** |
||
| 257 | * @return string |
||
| 258 | */ |
||
| 259 | public function getName() |
||
| 263 | |||
| 264 | /** |
||
| 265 | * @param string $type |
||
| 266 | * @return mixed |
||
| 267 | * @throws InvalidConfigException |
||
| 268 | */ |
||
| 269 | View Code Duplication | public function getResourceValue($type) |
|
| 279 | |||
| 280 | /** |
||
| 281 | * @param string $type |
||
| 282 | * @return string |
||
| 283 | * @throws InvalidConfigException |
||
| 284 | */ |
||
| 285 | View Code Duplication | public function getResourceTitle($type) |
|
| 294 | |||
| 295 | /** |
||
| 296 | * @param $type |
||
| 297 | * @return array|null |
||
| 298 | * @throws InvalidConfigException |
||
| 299 | */ |
||
| 300 | View Code Duplication | public function getOverusePrice($type) |
|
| 310 | |||
| 311 | /** |
||
| 312 | * @param string $type |
||
| 313 | * @return Part|null |
||
| 314 | */ |
||
| 315 | public function getPartByType($type) |
||
| 325 | |||
| 326 | /** |
||
| 327 | * @param string $type |
||
| 328 | * @return Resource|null |
||
| 329 | */ |
||
| 330 | public function getResourceByType($type) |
||
| 340 | |||
| 341 | /** |
||
| 342 | * @param string $type |
||
| 343 | * @return Resource|null |
||
| 344 | */ |
||
| 345 | public function getResourceByModelType($type) |
||
| 355 | |||
| 356 | /** |
||
| 357 | * @return array |
||
| 358 | */ |
||
| 359 | public function getLocations() |
||
| 365 | |||
| 366 | /** |
||
| 367 | * @return string |
||
| 368 | */ |
||
| 369 | public function getId() |
||
| 377 | } |
||
| 378 |
Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.
The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.
This check looks for comments that seem to be mostly valid code and reports them.