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 ParentWishlist 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 ParentWishlist, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
24 | class ParentWishlist extends MY_Controller |
||
25 | { |
||
26 | |||
27 | /** |
||
28 | * array that contains wishlist settings |
||
29 | * @var array |
||
30 | */ |
||
31 | public $settings = []; |
||
32 | |||
33 | /** |
||
34 | * contains output data |
||
35 | * @var mixed |
||
36 | */ |
||
37 | public $dataModel; |
||
38 | |||
39 | /** |
||
40 | * contains errors array |
||
41 | * @var array |
||
42 | */ |
||
43 | public $errors = []; |
||
44 | |||
45 | /** |
||
46 | * contains array of user wish products |
||
47 | * @var array |
||
48 | */ |
||
49 | public $userWishProducts; |
||
50 | |||
51 | public function __construct() { |
||
66 | |||
67 | /** |
||
68 | * set in cookie previous page url |
||
69 | * |
||
70 | * @access private |
||
71 | * @author DevImageCms |
||
72 | * @copyright (c) 2013, ImageCMS |
||
73 | */ |
||
74 | private function writeCookies() { |
||
86 | |||
87 | /** |
||
88 | * get all users wish lists |
||
89 | * |
||
90 | * @access public |
||
91 | * @param array $access - list access |
||
92 | * @author DevImageCms |
||
93 | * @copyright (c) 2013, ImageCMS |
||
94 | * @return boolean |
||
95 | */ |
||
96 | public function all($access = ['public']) { |
||
118 | |||
119 | /** |
||
120 | * get user wish list |
||
121 | * |
||
122 | * @access public |
||
123 | * @param integer $hash |
||
124 | * @param array $access list access |
||
125 | * @author DevImageCms |
||
126 | * @copyright (c) 2013, ImageCMS |
||
127 | * @return boolean |
||
128 | */ |
||
129 | public function show($hash, $access = ['shared', 'private', 'public']) { |
||
153 | |||
154 | /** |
||
155 | * add view point to list |
||
156 | * |
||
157 | * @access public |
||
158 | * @param integer $hash |
||
159 | * @author DevImageCms |
||
160 | * @copyright (c) 2013, ImageCMS |
||
161 | * @return boolean |
||
162 | */ |
||
163 | public static function addReview($hash) { |
||
186 | |||
187 | /** |
||
188 | * get most viewed wish list |
||
189 | * |
||
190 | * @access public |
||
191 | * @param integer $limit count lists to get |
||
192 | * @author DevImageCms |
||
193 | * @copyright (c) 2013, ImageCMS |
||
194 | * @return boolean |
||
195 | */ |
||
196 | public function getMostViewedWishLists($limit = 10) { |
||
206 | |||
207 | /** |
||
208 | * render user lists |
||
209 | * |
||
210 | * @access public |
||
211 | * @param integer $user_id |
||
212 | * @param array $access |
||
213 | * @author DevImageCms |
||
214 | * @copyright (c) 2013, ImageCMS |
||
215 | * @return boolean |
||
216 | */ |
||
217 | public function user($user_id, $access = ['public']) { |
||
226 | |||
227 | /** |
||
228 | * update user information |
||
229 | * |
||
230 | * @access public |
||
231 | * @param integer $userID |
||
232 | * @param string $user_name |
||
233 | * @param string $user_birthday |
||
234 | * @param string $description |
||
235 | * @author DevImageCms |
||
236 | * @copyright (c) 2013, ImageCMS |
||
237 | * @return boolean |
||
238 | */ |
||
239 | public function userUpdate($userID, $user_name, $user_birthday, $description) { |
||
247 | |||
248 | /** |
||
249 | * update wish list |
||
250 | * |
||
251 | * @access public |
||
252 | * @param integer $id |
||
253 | * @param array $data |
||
254 | * @param array $comments |
||
255 | * @author DevImageCms |
||
256 | * @copyright (c) 2013, ImageCMS |
||
257 | * @return boolean |
||
258 | */ |
||
259 | public function updateWL($id, $data, $comments) { |
||
271 | |||
272 | /** |
||
273 | * create wish list |
||
274 | * |
||
275 | * @access public |
||
276 | * @param integer $user_id |
||
277 | * @param string $listName |
||
278 | * @param string $wlType |
||
279 | * @param string $wlDescription |
||
280 | * @return bool |
||
281 | * @author DevImageCms |
||
282 | * @copyright (c) 2013, ImageCMS |
||
283 | */ |
||
284 | public function createWishList($user_id, $listName, $wlType, $wlDescription) { |
||
318 | |||
319 | /** |
||
320 | * delete full WL |
||
321 | * |
||
322 | * @access public |
||
323 | * @param integer $id list id |
||
324 | * @author DevImageCms |
||
325 | * @copyright (c) 2013, ImageCMS |
||
326 | * @return boolean |
||
327 | */ |
||
328 | public function deleteWL($id) { |
||
344 | |||
345 | /** |
||
346 | * delete all wishlists |
||
347 | * @param integer $UserID |
||
348 | * @return boolean |
||
349 | */ |
||
350 | public function deleteAllWL($UserID) { |
||
374 | |||
375 | /** |
||
376 | * add item to wish list |
||
377 | * |
||
378 | * @access public |
||
379 | * @param integer $varId |
||
380 | * @param string $listId |
||
381 | * @param string $listName |
||
382 | * @param integer $userId |
||
383 | * @author DevImageCms |
||
384 | * @copyright (c) 2013, ImageCMS |
||
385 | * @return boolean |
||
386 | */ |
||
387 | public function _addItem($varId, $listId, $listName, $userId = null) { |
||
435 | |||
436 | /** |
||
437 | * move item from one wish list to another |
||
438 | * |
||
439 | * @param integer $varId |
||
440 | * @param integer $wish_list_id |
||
441 | * @param int|string $to_listId |
||
442 | * @param int|string $to_listName |
||
443 | * @param null $user_id |
||
444 | * @return bool |
||
445 | * @access public |
||
446 | * @author DevImageCms |
||
447 | * @copyright (c) 2013, ImageCMS |
||
448 | */ |
||
449 | public function moveItem($varId, $wish_list_id, $to_listId = '', $to_listName = '', $user_id = null) { |
||
462 | |||
463 | /** |
||
464 | * delete item from wish list |
||
465 | * |
||
466 | * @param integer $variant_id |
||
467 | * @param integer $wish_list_id |
||
468 | * @access public |
||
469 | * @author DevImageCms |
||
470 | * @copyright (c) 2013, ImageCMS |
||
471 | * @return boolean |
||
472 | */ |
||
473 | public function deleteItem($variant_id, $wish_list_id) { |
||
483 | |||
484 | /** |
||
485 | * get user info |
||
486 | * |
||
487 | * @param integer $id |
||
488 | * @access public |
||
489 | * @author DevImageCms |
||
490 | * @copyright (c) 2013, ImageCMS |
||
491 | * @return boolean |
||
492 | */ |
||
493 | public function getUserInfo($id) { |
||
496 | |||
497 | /** |
||
498 | * render user wish list |
||
499 | * |
||
500 | * @param integer $userId |
||
501 | * @param array $access |
||
502 | * @access public |
||
503 | * @author DevImageCms |
||
504 | * @copyright (c) 2013, ImageCMS |
||
505 | * @return boolean |
||
506 | */ |
||
507 | public function getUserWL($userId, $access = ['public', 'private', 'shared']) { |
||
532 | |||
533 | /** |
||
534 | * render user wish list edit page |
||
535 | * |
||
536 | * @param integer $wish_list_id |
||
537 | * @param integer $userID |
||
538 | * @access public |
||
539 | * @author DevImageCms |
||
540 | * @copyright (c) 2013, ImageCMS |
||
541 | * @return boolean |
||
542 | */ |
||
543 | public function renderUserWLEdit($wish_list_id, $userID = null) { |
||
563 | |||
564 | /** |
||
565 | * upload image for user |
||
566 | * |
||
567 | * @param integer $userID |
||
568 | * @access public |
||
569 | * @author DevImageCms |
||
570 | * @copyright (c) 2013, ImageCMS |
||
571 | * @return boolean |
||
572 | */ |
||
573 | public function do_upload($userID = null) { |
||
618 | |||
619 | /** |
||
620 | * get most popular items by wish list usage |
||
621 | * |
||
622 | * @param integer $limit |
||
623 | * @access public |
||
624 | * @author DevImageCms |
||
625 | * @copyright (c) 2013, ImageCMS |
||
626 | * @return boolean |
||
627 | */ |
||
628 | public function getMostPopularItems($limit = 10) { |
||
639 | |||
640 | /** |
||
641 | * get user wish list items count |
||
642 | * |
||
643 | * @param integer $user_id |
||
644 | * @access public |
||
645 | * @author DevImageCms |
||
646 | * @copyright (c) 2013, ImageCMS |
||
647 | * @return integer |
||
648 | */ |
||
649 | public function getUserWishListItemsCount($user_id) { |
||
652 | |||
653 | /** |
||
654 | * delete list items by id's |
||
655 | * |
||
656 | * @param array $ids |
||
657 | * @access public |
||
658 | * @author DevImageCms |
||
659 | * @copyright (c) 2013, ImageCMS |
||
660 | * @return boolean |
||
661 | */ |
||
662 | public function deleteItemsByIds($ids) { |
||
665 | |||
666 | /** |
||
667 | * delete image |
||
668 | * |
||
669 | * @param string $image image name |
||
670 | * @access public |
||
671 | * @author DevImageCms |
||
672 | * @copyright (c) 2013, ImageCMS |
||
673 | * @return boolean |
||
674 | */ |
||
675 | public function deleteImage($image, $user_id) { |
||
680 | |||
681 | /** |
||
682 | * get popup for adding or moving items |
||
683 | * |
||
684 | * @param integer $userID |
||
685 | * @access public |
||
686 | * @author DevImageCms |
||
687 | * @copyright (c) 2013, ImageCMS |
||
688 | * @return boolean |
||
689 | */ |
||
690 | public function renderPopup($userID = null) { |
||
699 | |||
700 | /** |
||
701 | * |
||
702 | * @param integer $wish_list_id |
||
703 | * @param string $email |
||
704 | * @return boolean |
||
705 | */ |
||
706 | public function send_email($wish_list_id, $email) { |
||
736 | |||
737 | public function autoload() { |
||
740 | |||
741 | public static function adminAutoload() { |
||
744 | |||
745 | public function _install() { |
||
748 | |||
749 | public function _deinstall() { |
||
752 | |||
753 | } |
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
$accountId
that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to theid
property of an instance of theAccount
class. 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.