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 Product 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 Product, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
14 | class Product extends Intraface_Standard |
||
15 | { |
||
16 | /** |
||
17 | * @var object |
||
18 | */ |
||
19 | public $kernel; |
||
20 | |||
21 | /** |
||
22 | * @var object |
||
23 | */ |
||
24 | public $user; |
||
25 | |||
26 | /** |
||
27 | * @var integer |
||
28 | */ |
||
29 | private $id; |
||
30 | |||
31 | /** |
||
32 | * @var object |
||
33 | */ |
||
34 | private $detail; |
||
35 | |||
36 | /** |
||
37 | * @var integer |
||
38 | */ |
||
39 | private $old_product_detail_id; |
||
40 | |||
41 | /** |
||
42 | * @var array |
||
43 | */ |
||
44 | public $value = array(); |
||
45 | |||
46 | /** |
||
47 | * Fields to update |
||
48 | * @var array |
||
49 | */ |
||
50 | private $fields; |
||
51 | |||
52 | /** |
||
53 | * @var object |
||
54 | */ |
||
55 | private $db; |
||
56 | |||
57 | /** |
||
58 | * @var object |
||
59 | * |
||
60 | * Made private now. Please use getStock() instead. |
||
61 | */ |
||
62 | private $stock; |
||
63 | |||
64 | /** |
||
65 | * @var object |
||
66 | */ |
||
67 | public $error; |
||
68 | |||
69 | /** |
||
70 | * @var object |
||
71 | */ |
||
72 | public $keywords; |
||
73 | |||
74 | /** |
||
75 | * @var object |
||
76 | */ |
||
77 | private $dbquery; |
||
78 | |||
79 | /** |
||
80 | * Constructor |
||
81 | * |
||
82 | * @param object $kernel The kernel object |
||
83 | * @param integer $product_id The product id |
||
84 | * @param integer $old_product_detail_id If we want to find old details, e.g. for an invoice |
||
85 | * |
||
86 | * @return void |
||
|
|||
87 | */ |
||
88 | 67 | function __construct($kernel, $product_id = 0, $old_product_detail_id = 0) |
|
103 | |||
104 | /** |
||
105 | * Used by Keyword |
||
106 | * |
||
107 | * @see Keyword |
||
108 | * |
||
109 | * @return string |
||
110 | */ |
||
111 | 1 | function identify() |
|
115 | |||
116 | /** |
||
117 | * Creates the dbquery object |
||
118 | |||
119 | * @return void |
||
120 | */ |
||
121 | public function getDBQuery() |
||
138 | |||
139 | /** |
||
140 | * Loads data into an array |
||
141 | * |
||
142 | * Should load both the specific product details, but also the product details |
||
143 | * |
||
144 | * @return integer product id or 0 |
||
145 | */ |
||
146 | 66 | public function load() |
|
194 | |||
195 | /** |
||
196 | * Gets the stock module |
||
197 | * |
||
198 | * @return object |
||
199 | */ |
||
200 | 8 | public function getStock() |
|
222 | |||
223 | 1 | function getKernel() |
|
227 | |||
228 | /** |
||
229 | * @todo How should we handle the function to get pictures? |
||
230 | */ |
||
231 | 18 | function getNewPictures() |
|
232 | { |
||
233 | require_once 'Intraface/modules/filemanager/AppendFile.php'; |
||
234 | //$filehandler = new Ilib_Filehandler($this->kernel); |
||
235 | $append_file = new AppendFile($this->kernel, 'product', $this->get('id')); |
||
236 | $appendix_list = $append_file->getList(); |
||
237 | |||
238 | 18 | $this->value['pictures'] = array(); |
|
239 | |||
240 | View Code Duplication | if (count($appendix_list) > 0) { |
|
241 | foreach ($appendix_list as $key => $appendix) { |
||
242 | $tmp_filehandler = new Ilib_Filehandler($this->kernel, $appendix['file_handler_id']); |
||
243 | $this->value['pictures'][$key]['id'] = $appendix['file_handler_id']; |
||
244 | $this->value['pictures'][$key]['original']['icon_uri'] = $tmp_filehandler->get('icon_uri'); |
||
245 | $this->value['pictures'][$key]['original']['name'] = $tmp_filehandler->get('file_name'); |
||
246 | $this->value['pictures'][$key]['original']['width'] = $tmp_filehandler->get('width'); |
||
247 | $this->value['pictures'][$key]['original']['height'] = $tmp_filehandler->get('height'); |
||
248 | $this->value['pictures'][$key]['original']['file_uri'] = $tmp_filehandler->get('file_uri'); |
||
249 | $this->value['pictures'][$key]['appended_file_id'] = $appendix['id']; |
||
250 | |||
251 | if ($tmp_filehandler->get('is_image')) { |
||
252 | $tmp_filehandler->createInstance(); |
||
253 | $instances = $tmp_filehandler->instance->getList('include_hidden'); |
||
254 | foreach ($instances as $instance) { |
||
255 | $this->value['pictures'][$key][$instance['name']]['file_uri'] = $instance['file_uri']; |
||
256 | $this->value['pictures'][$key][$instance['name']]['name'] = $instance['name']; |
||
257 | $this->value['pictures'][$key][$instance['name']]['width'] = $instance['width']; |
||
258 | $this->value['pictures'][$key][$instance['name']]['height'] = $instance['height']; |
||
259 | } |
||
260 | } |
||
261 | } |
||
262 | } |
||
263 | return $this->value['pictures']; |
||
264 | } |
||
265 | |||
266 | /** |
||
267 | * Gets pictures for the product |
||
268 | * |
||
269 | * @return array |
||
270 | */ |
||
271 | 19 | function getPictures() |
|
309 | |||
310 | /** |
||
311 | * Validates |
||
312 | * |
||
313 | * @param array $array_var The array to validate |
||
314 | * |
||
315 | * @return boolean |
||
316 | */ |
||
317 | 66 | private function validate($array_var) |
|
341 | |||
342 | /** |
||
343 | * Saves product |
||
344 | * |
||
345 | * @param array $array_var Array with details to save, @see $this->fields |
||
346 | * |
||
347 | * @return integer 0 on error |
||
348 | */ |
||
349 | 66 | public function save($array_var) |
|
424 | |||
425 | /** |
||
426 | * Copies product to a new product |
||
427 | * |
||
428 | * @return integer Id for the new product |
||
429 | */ |
||
430 | 1 | public function copy() |
|
484 | |||
485 | /** |
||
486 | * Deletes product |
||
487 | * |
||
488 | * Only set active to 0. Products must never be deleted from the database. It should always be |
||
489 | * possible to go back to earlier products. |
||
490 | * |
||
491 | * @return boolean |
||
492 | */ |
||
493 | 30 | public function delete() |
|
515 | |||
516 | /** |
||
517 | * Undeletes a product |
||
518 | * |
||
519 | * @return boolean |
||
520 | */ |
||
521 | 1 | public function undelete() |
|
537 | |||
538 | /** |
||
539 | * Returnerer det h�jeste produktnummer |
||
540 | * |
||
541 | * @return integer produktnummer |
||
542 | */ |
||
543 | 66 | View Code Duplication | public function getMaxNumber() |
558 | |||
559 | /** |
||
560 | * Checks whether number is free |
||
561 | * |
||
562 | * @param integer $product_number Product number to check |
||
563 | * |
||
564 | * @return boolean |
||
565 | */ |
||
566 | 66 | public function isNumberFree($product_number) |
|
586 | |||
587 | /** |
||
588 | * Get keywords object |
||
589 | * |
||
590 | * @deprecated |
||
591 | * |
||
592 | * @return object |
||
593 | */ |
||
594 | public function getKeywords() |
||
598 | |||
599 | /** |
||
600 | * Get keywords object |
||
601 | * |
||
602 | * @return object |
||
603 | */ |
||
604 | public function getKeyword() |
||
605 | { |
||
606 | return ($this->keywords = new Keyword($this)); |
||
607 | } |
||
608 | |||
609 | |||
610 | 1 | public function getKeywordAppender() |
|
611 | { |
||
612 | 1 | return new Intraface_Keyword_Appender($this); |
|
613 | } |
||
614 | |||
615 | /** |
||
616 | * Set related product |
||
617 | * |
||
618 | * @param integer $id Product id to relate to this product |
||
619 | * @param string $status Can be relate or remove |
||
620 | * |
||
621 | * @return boolean |
||
622 | */ |
||
623 | 1 | public function setRelatedProduct($id, $status = 'relate') |
|
646 | |||
647 | /** |
||
648 | * Delete related product |
||
649 | * |
||
650 | * @param integer $id Product id to relate to this product |
||
651 | * |
||
652 | * @return boolean |
||
653 | */ |
||
654 | public function deleteRelatedProduct($id) |
||
655 | { |
||
656 | $db = new DB_Sql; |
||
657 | $db->query("DELETE FROM product_related WHERE product_id = " . $this->id . " AND intranet_id = " . $this->intranet->getId() . " AND related_product_id = " . (int)$id); |
||
658 | return true; |
||
659 | } |
||
660 | |||
661 | /** |
||
662 | * Delete all related product |
||
663 | * |
||
664 | * @param integer $id Product id to relate to this product |
||
665 | * |
||
666 | * @return boolean |
||
667 | */ |
||
668 | public function deleteRelatedProducts() |
||
669 | { |
||
670 | $db = new DB_Sql; |
||
671 | $db->query("DELETE FROM product_related WHERE product_id = " . $this->id . " AND intranet_id = " . $this->intranet->getId()); |
||
672 | return true; |
||
673 | } |
||
674 | |||
675 | /** |
||
676 | * Get all related products |
||
677 | * |
||
678 | * @return array |
||
679 | */ |
||
680 | 2 | public function getRelatedProducts($currencies = false, $show = 'all') |
|
681 | { |
||
682 | 2 | $products = array(); |
|
683 | 2 | $key = 0; |
|
684 | 2 | $ids = array(); |
|
685 | 2 | $db = new DB_Sql; |
|
686 | 2 | $sql = "SELECT related_product_id FROM product_related WHERE product_id = " . $this->id . " AND intranet_id = " . $this->intranet->getId(); |
|
687 | 2 | $db->query($sql); |
|
688 | |||
689 | // r�kkef�lgen er vigtig - f�rst hente fra product og bagefter tilf�je nye v�rdier til arrayet |
||
690 | 2 | while ($db->nextRecord()) { |
|
691 | 1 | $product = new Product($this->kernel, $db->f('related_product_id')); |
|
692 | 1 | if ($product->get('id') == 0 || $product->get('active') == 0 || ($show == 'webshop' && $product->get('do_show') == 0)) { |
|
693 | continue; |
||
694 | } |
||
695 | 1 | $products[$key] = $product->get(); |
|
696 | 1 | $products[$key]['related_id'] = $db->f('related_product_id'); |
|
697 | |||
698 | 1 | $products[$key]['currency']['DKK']['price'] = $product->getDetails()->getPrice(); |
|
699 | 1 | $products[$key]['currency']['DKK']['price_incl_vat'] = $product->getDetails()->getPriceIncludingVat(); |
|
700 | 1 | $products[$key]['currency']['DKK']['before_price'] = $product->getDetails()->getBeforePrice(); |
|
701 | 1 | $products[$key]['currency']['DKK']['before_price_incl_vat'] = $product->getDetails()->getBeforePriceIncludingVat(); |
|
702 | |||
703 | 1 | $products[$key]['pictures'] = $product->getPictures(); |
|
704 | |||
705 | 1 | View Code Duplication | if ($currencies && $currencies->count() > 0) { |
706 | foreach ($currencies as $currency) { |
||
707 | $products[$key]['currency'][$currency->getType()->getIsoCode()]['price'] = $product->getDetails()->getPriceInCurrency($currency); |
||
708 | $products[$key]['currency'][$currency->getType()->getIsoCode()]['price_incl_vat'] = $product->getDetails()->getPriceIncludingVatInCurrency($currency); |
||
709 | $products[$key]['currency'][$currency->getType()->getIsoCode()]['before_price'] = $product->getDetails()->getBeforePriceInCurrency($currency); |
||
710 | $products[$key]['currency'][$currency->getType()->getIsoCode()]['before_price_incl_vat'] = $product->getDetails()->getBeforePriceIncludingVatInCurrency($currency); |
||
711 | } |
||
712 | } |
||
713 | |||
714 | 1 | if (!$product->hasVariation() and is_object($product->getStock())) { |
|
715 | $products[$key]['stock_status'] = $product->getStock()->get(); |
||
716 | View Code Duplication | } else { |
|
717 | // alle ikke lagervarer der skal vises i webshop skal have en for_sale |
||
718 | 1 | if ($product->get('stock') == 0 and $product->get('do_show') == 1) { |
|
719 | 1 | $products[$key]['stock_status'] = array('for_sale' => 100); // kun til at stock_status |
|
720 | 1 | } else { |
|
721 | $products[$key]['stock_status'] = array(); |
||
722 | } |
||
723 | } |
||
724 | // den her skal vist lige kigges igennem, for den tager jo alt med p� nettet? |
||
725 | // 0 = only stock |
||
726 | 1 | View Code Duplication | if ($this->kernel->setting->get('intranet', 'webshop.show_online') == 0 and !empty($which) and $which=='webshop') { // only stock |
727 | if (array_key_exists('for_sale', $products[$key]['stock_status']) and $products[$key]['stock_status']['for_sale'] <= 0) { |
||
728 | continue; |
||
729 | } |
||
730 | } |
||
731 | 1 | $key++; |
|
732 | 1 | } |
|
733 | 2 | return $products; |
|
734 | } |
||
735 | |||
736 | 21 | function hasVariation() |
|
737 | { |
||
738 | 21 | return $this->get('has_variation'); |
|
739 | } |
||
740 | |||
741 | /** |
||
742 | * Set attribute for product |
||
743 | * |
||
744 | * @param integer $id Attribute id to relate to this product |
||
745 | * |
||
746 | * @return boolean |
||
747 | */ |
||
748 | 9 | public function setAttributeGroup($id) |
|
749 | { |
||
750 | 9 | if (!$this->get('has_variation')) { |
|
751 | 1 | throw new Exception('You can not set attribute group for a product without variations!'); |
|
752 | } |
||
753 | |||
754 | 8 | $db = MDB2::singleton(DB_DSN); |
|
755 | 8 | $result = $db->query("SELECT id FROM product_x_attribute_group WHERE intranet_id = ".$db->quote($this->intranet->getId())." AND product_id=" . $this->getId() . " AND product_attribute_group_id = " . (int)$id); |
|
756 | 8 | if (PEAR::isError($result)) { |
|
757 | throw new Exception('Error in query :'.$result->getUserInfo()); |
||
758 | } |
||
759 | |||
760 | 8 | if ($result->numRows() > 0) { |
|
761 | return true; |
||
762 | } |
||
763 | 8 | $result = $db->exec("INSERT INTO product_x_attribute_group SET product_id = " . $this->getId() . ", product_attribute_group_id = " . (int)$id . ", intranet_id = " . $this->intranet->getId()); |
|
764 | 8 | if (PEAR::isError($result)) { |
|
765 | throw new Exception('Error in insert :'.$result->getUserInfo()); |
||
766 | } |
||
767 | |||
768 | 8 | return true; |
|
769 | } |
||
770 | |||
771 | /** |
||
772 | * Remove attribute for product |
||
773 | * |
||
774 | * @param integer $id Attribute id to relate to this product |
||
775 | * |
||
776 | * @return boolean |
||
777 | */ |
||
778 | 1 | public function removeAttributeGroup($id) |
|
779 | { |
||
780 | 1 | if (!$this->get('has_variation')) { |
|
781 | throw new Exception('You can not remove attribute group for a product without variations!'); |
||
782 | } |
||
783 | |||
784 | 1 | $db = MDB2::singleton(DB_DSN); |
|
785 | 1 | $result = $db->exec("DELETE FROM product_x_attribute_group WHERE intranet_id = ".$db->quote($this->intranet->getId())." AND product_id=" . $this->getId() . " AND product_attribute_group_id = " . (int)$id); |
|
786 | 1 | if (PEAR::isError($result)) { |
|
787 | throw new Exception('Error in query :'.$result->getUserInfo()); |
||
788 | } |
||
789 | |||
790 | 1 | return ($result > 0); |
|
791 | } |
||
792 | |||
793 | /** |
||
794 | * Get all attributes related to the product |
||
795 | * |
||
796 | * @todo Rewrite product_x_attribute_group to Doctrine. |
||
797 | * @todo Add a field named attribute_number to product_x_attribute_group, to be sure |
||
798 | * that a attribute always relates to the correct attribute number on the variation. |
||
799 | * |
||
800 | * @return array |
||
801 | */ |
||
802 | 7 | public function getAttributeGroups() |
|
803 | { |
||
804 | 7 | if (!$this->get('has_variation')) { |
|
805 | throw new Exception('You can not get attribute groups for a product without variations!'); |
||
806 | } |
||
807 | |||
808 | // takes groups despite the are deleted. That is probably the best behaviour for now |
||
809 | // NOTE: Very important that it is ordered by product_attribute_group.id so the groups |
||
810 | // does always get attached to the correct attribute number on the variation. Se above todo in method doc |
||
811 | 7 | $db = MDB2::singleton(DB_DSN); |
|
812 | 7 | $result = $db->query("SELECT product_attribute_group.* FROM product_x_attribute_group " . |
|
813 | 7 | "INNER JOIN product_attribute_group " . |
|
814 | 7 | "ON product_x_attribute_group.product_attribute_group_id = product_attribute_group.id " . |
|
815 | 7 | "AND product_attribute_group.intranet_id = ".$db->quote($this->intranet->getId())." " . |
|
816 | 7 | "WHERE product_x_attribute_group.intranet_id = ".$db->quote($this->intranet->getId())." " . |
|
817 | 7 | "AND product_x_attribute_group.product_id=" . $this->getId() . " " . |
|
818 | 7 | "ORDER BY product_attribute_group.id"); |
|
819 | |||
820 | 7 | if (PEAR::isError($result)) { |
|
821 | throw new Exception('Error in query :'.$result->getUserInfo()); |
||
822 | } |
||
823 | |||
824 | 7 | return $result->fetchAll(MDB2_FETCHMODE_ASSOC); |
|
825 | } |
||
826 | |||
827 | /** |
||
828 | * returns variation |
||
829 | */ |
||
830 | 6 | View Code Duplication | public function getVariation($id = 0) |
831 | { |
||
832 | 6 | $gateway = new Intraface_modules_product_Variation_Gateway($this); |
|
833 | 5 | if (intval($id) > 0) { |
|
834 | 3 | return $gateway->findById($id); |
|
835 | } |
||
836 | 5 | $object = $gateway->getObject(); |
|
837 | 5 | unset($gateway); |
|
838 | 5 | $object->product_id = $this->getId(); |
|
839 | 5 | return $object; |
|
840 | } |
||
841 | |||
842 | /** |
||
843 | * Returns variation on the basis of attributes |
||
844 | * |
||
845 | * @param array $attributes Attributes to find variation from |
||
846 | * array('attribte1' => [id1], 'attribute2' => [id2]); |
||
847 | */ |
||
848 | public function getVariationFromAttributes($attributes) |
||
849 | { |
||
850 | $gateway = new Intraface_modules_product_Variation_Gateway($this); |
||
851 | return $gateway->findByAttributes($attributes); |
||
852 | } |
||
853 | |||
854 | /** |
||
855 | * Returns all variations on product |
||
856 | */ |
||
857 | 1 | public function getVariations() |
|
858 | { |
||
859 | 1 | $gateway = new Intraface_modules_product_Variation_Gateway($this); |
|
860 | 1 | return $gateway->findAll(); |
|
861 | } |
||
862 | |||
863 | /** |
||
864 | * Checks whether any products has been created before |
||
865 | * |
||
866 | * @return integer |
||
867 | */ |
||
868 | 1 | View Code Duplication | public function isFilledIn() |
869 | { |
||
870 | 1 | $db = new DB_Sql; |
|
871 | 1 | $db->query("SELECT count(*) AS antal FROM product WHERE intranet_id = " . $this->intranet->getId()); |
|
872 | 1 | if ($db->nextRecord()) { |
|
873 | 1 | return $db->f('antal'); |
|
874 | } |
||
875 | return 0; |
||
876 | } |
||
877 | |||
878 | /** |
||
879 | * Checks whether there is any active products. Differs from isFilledIn() by checking for active = 1 |
||
880 | * |
||
881 | * @return integer |
||
882 | */ |
||
883 | public function any() |
||
884 | { |
||
885 | $db = new DB_Sql; |
||
886 | $db->query("SELECT id FROM product WHERE intranet_id = " . $this->intranet->getId()." AND active = 1"); |
||
887 | return $db->numRows(); |
||
888 | } |
||
889 | |||
890 | 2 | public function isActive() |
|
891 | { |
||
892 | 2 | if ($this->value['active'] == 0) { |
|
893 | 2 | return false; |
|
894 | } |
||
895 | |||
896 | 1 | return true; |
|
897 | } |
||
898 | |||
899 | /** |
||
900 | * Public: Finde data til en liste |
||
901 | * |
||
902 | * Hvis den er fra webshop b�r den faktisk opsamle oplysninger om s�gningen |
||
903 | * s� man kan se, hvad folk er interesseret i. |
||
904 | * S�gemaskinen skal v�re tolerant for stavefejl |
||
905 | * |
||
906 | * @todo It is wrong to give currencies as parameter. Instead the list should |
||
907 | * be given as an object collection, and then currency should be given |
||
908 | * to the getPrice method. |
||
909 | * |
||
910 | * @param string $which valgfri s�geparameter - ikke aktiv endnu |
||
911 | * @param object $currencies Collection of valid currencies. |
||
912 | * @return array indeholdende kundedata til liste |
||
913 | */ |
||
914 | function getList($which = 'all', $currencies = false) |
||
1030 | |||
1031 | /** |
||
1032 | * Gets id |
||
1033 | * |
||
1034 | * @return integer |
||
1035 | */ |
||
1036 | 27 | function getId() |
|
1037 | { |
||
1038 | 27 | return $this->id; |
|
1039 | } |
||
1040 | |||
1041 | /** |
||
1042 | * Gets the details |
||
1043 | * |
||
1044 | * @return object |
||
1045 | */ |
||
1046 | 66 | function getDetails() |
|
1047 | { |
||
1050 | |||
1051 | /** |
||
1052 | * returns the possible units |
||
1053 | * |
||
1054 | * @return array units |
||
1055 | */ |
||
1056 | public static function getUnits() |
||
1060 | } |
||
1061 |
Adding a
@return
annotation to a constructor is not recommended, since a constructor does not have a meaningful return value.Please refer to the PHP core documentation on constructors.