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 VisitorModel 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 VisitorModel, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 18 | class VisitorModel extends BaseModel |
||
| 19 | { |
||
| 20 | |||
| 21 | /** @var string search pattern */ |
||
| 22 | public $search; |
||
| 23 | |||
| 24 | /** @var Meeting Meeting class */ |
||
| 25 | public $Meeting; |
||
| 26 | |||
| 27 | /** @var Meal Meals class */ |
||
| 28 | public $Meals; |
||
| 29 | |||
| 30 | /** @var Program Programs class */ |
||
| 31 | public $Programs; |
||
| 32 | |||
| 33 | /** @var Blocks Blocks class */ |
||
| 34 | public $Blocks; |
||
| 35 | |||
| 36 | /** @var int meeting price */ |
||
| 37 | public $meeting_price; |
||
| 38 | |||
| 39 | /** @var int meeting advance */ |
||
| 40 | private $meeting_advance; |
||
| 41 | |||
| 42 | /** |
||
| 43 | * Array of database programs table columns |
||
| 44 | * |
||
| 45 | * @var array dbColumns[] |
||
| 46 | */ |
||
| 47 | public $dbColumns = array(); |
||
| 48 | |||
| 49 | /** |
||
| 50 | * Array of form names |
||
| 51 | * |
||
| 52 | * @var array formNames[] |
||
| 53 | */ |
||
| 54 | public $formNames = array(); |
||
| 55 | |||
| 56 | protected $table = 'kk_visitors'; |
||
| 57 | |||
| 58 | protected $columns = [ |
||
| 59 | 'name', |
||
| 60 | 'surname', |
||
| 61 | 'nick', |
||
| 62 | 'email', |
||
| 63 | 'birthday', |
||
| 64 | 'street', |
||
| 65 | 'city', |
||
| 66 | 'postal_code', |
||
| 67 | 'group_num', |
||
| 68 | 'group_name', |
||
| 69 | 'troop_name', |
||
| 70 | 'province', |
||
| 71 | 'arrival', |
||
| 72 | 'departure', |
||
| 73 | 'comment', |
||
| 74 | 'question', |
||
| 75 | 'question2', |
||
| 76 | 'bill', |
||
| 77 | 'cost', |
||
| 78 | 'meeting', |
||
| 79 | ]; |
||
| 80 | |||
| 81 | /** konstruktor */ |
||
| 82 | public function __construct( |
||
| 123 | |||
| 124 | /** |
||
| 125 | * @return array |
||
| 126 | */ |
||
| 127 | public function getColumns() |
||
| 131 | |||
| 132 | /** |
||
| 133 | * Create a new visitor |
||
| 134 | * |
||
| 135 | * @return string |
||
| 136 | */ |
||
| 137 | public function assemble(array $DB_data, $meals_data, $programs_data, $returnGuid = false) |
||
| 193 | |||
| 194 | /** |
||
| 195 | * Modify a visitor |
||
| 196 | * |
||
| 197 | * @param int $visitor_id ID of a visitor |
||
| 198 | * @param array $db_data Visitor's database data |
||
| 199 | * @param array $meals_data Data of meals |
||
| 200 | * @param array $programs_data Program's data |
||
| 201 | * @return mixed TRUE or array of errors |
||
| 202 | */ |
||
| 203 | public function modify($ID_visitor, $DB_data, $meals_data, $programs_data) |
||
| 239 | |||
| 240 | /** |
||
| 241 | * Modify a visitor |
||
| 242 | * |
||
| 243 | * @param int $visitor_id ID of a visitor |
||
| 244 | * @param array $db_data Visitor's database data |
||
| 245 | * @param array $meals_data Data of meals |
||
| 246 | * @param array $programs_data Program's data |
||
| 247 | * @return mixed TRUE or array of errors |
||
| 248 | */ |
||
| 249 | public function modifyByGuid($guid, $DB_data, $meals_data, $programs_data) |
||
| 287 | |||
| 288 | /** |
||
| 289 | * Delete one or multiple record/s |
||
| 290 | * |
||
| 291 | * @param int ID/s of record |
||
| 292 | * @return boolean |
||
| 293 | */ |
||
| 294 | public function delete($id) |
||
| 303 | |||
| 304 | /** |
||
| 305 | * Set as checked one or multiple record/s |
||
| 306 | * |
||
| 307 | * @param int ID/s of record |
||
| 308 | * @param int 0 | 1 |
||
| 309 | * @return boolean |
||
| 310 | */ |
||
| 311 | public function checked($id, $value) |
||
| 320 | |||
| 321 | /** |
||
| 322 | * Get count of visitors |
||
| 323 | * |
||
| 324 | * @return integer |
||
| 325 | */ |
||
| 326 | public function getCount() |
||
| 333 | |||
| 334 | /** |
||
| 335 | * @param string $search |
||
| 336 | * @return $this |
||
| 337 | */ |
||
| 338 | public function setSearch($search) |
||
| 344 | |||
| 345 | /** |
||
| 346 | * @return string |
||
| 347 | */ |
||
| 348 | protected function getSearch() |
||
| 352 | |||
| 353 | /** |
||
| 354 | * @return string |
||
| 355 | */ |
||
| 356 | protected function buildSearchQuery() |
||
| 373 | |||
| 374 | /** |
||
| 375 | * Modify the visitor's bill |
||
| 376 | * |
||
| 377 | * @param int ID/s of visitor |
||
| 378 | * @param string type of payment (pay | advance) |
||
| 379 | * @return string error message or true |
||
| 380 | */ |
||
| 381 | public function payCharge($query_id, $type) |
||
| 397 | |||
| 398 | /** |
||
| 399 | * Get recipients by ids |
||
| 400 | * |
||
| 401 | * @param mixed ID of visitor |
||
| 402 | * @return mixed result |
||
| 403 | */ |
||
| 404 | public function getRecipients($ids) |
||
| 413 | |||
| 414 | /** |
||
| 415 | * Get visitor's programs |
||
| 416 | * |
||
| 417 | * @param int ID of visitor |
||
| 418 | * @return mixed result |
||
| 419 | */ |
||
| 420 | public function getVisitorPrograms($visitorId) |
||
| 428 | |||
| 429 | /** |
||
| 430 | * @param string|DateTime $datetime |
||
| 431 | * @return DateTime |
||
| 432 | */ |
||
| 433 | protected function convertToDateTime($datetime): DateTime |
||
| 441 | |||
| 442 | /** |
||
| 443 | * Render program switcher for unique visitor |
||
| 444 | * |
||
| 445 | * @param int ID of meeting |
||
| 446 | * @param int ID of visitor |
||
| 447 | * @return string html |
||
| 448 | */ |
||
| 449 | public function renderProgramSwitcher($meetingId, $visitorId) |
||
| 472 | |||
| 473 | /** |
||
| 474 | * @return Row |
||
| 475 | */ |
||
| 476 | public function all() |
||
| 499 | |||
| 500 | /** |
||
| 501 | * Return visitor by id |
||
| 502 | * |
||
| 503 | * @param int $id |
||
| 504 | * @return ActiveRow |
||
| 505 | */ |
||
| 506 | public function findById($id) |
||
| 510 | |||
| 511 | /** |
||
| 512 | * Return visitor by guid |
||
| 513 | * |
||
| 514 | * @param string $guid |
||
| 515 | * @return ActiveRow |
||
| 516 | */ |
||
| 517 | public function findByGuid($guid) |
||
| 521 | |||
| 522 | /** |
||
| 523 | * @param array $queryId |
||
| 524 | * @return string |
||
| 525 | */ |
||
| 526 | public function getSerializedMailAddress(array $queryId = []) |
||
| 545 | |||
| 546 | /** |
||
| 547 | * @param int $id |
||
| 548 | * @return Visitor |
||
| 549 | */ |
||
| 550 | public function getBill($id) |
||
| 558 | |||
| 559 | /** |
||
| 560 | * @param int ID of visitor |
||
| 561 | * @return mixed result |
||
| 562 | */ |
||
| 563 | public function findVisitorPrograms(int $visitorId) |
||
| 571 | |||
| 572 | } |
||
| 573 |
Our type inference engine has found an assignment to a property that is incompatible with the declared type of that property.
Either this assignment is in error or the assigned type should be added to the documentation/type hint for that property..