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 BasketHelper 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 BasketHelper, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 23 | class BasketHelper |
||
| 24 | { |
||
| 25 | const REMOTE_SHIPPING = 'remote'; |
||
| 26 | |||
| 27 | /** |
||
| 28 | * The basket array decorated by this class |
||
| 29 | * @var array |
||
| 30 | */ |
||
| 31 | protected $basket; |
||
| 32 | |||
| 33 | /** |
||
| 34 | * Array of connect product structs |
||
| 35 | * @var array |
||
| 36 | */ |
||
| 37 | protected $connectProducts = []; |
||
| 38 | |||
| 39 | /** |
||
| 40 | * connect content as formated by shopware |
||
| 41 | * |
||
| 42 | * @var array |
||
| 43 | */ |
||
| 44 | protected $connectContent = []; |
||
| 45 | |||
| 46 | /** |
||
| 47 | * Array of connect shops affected by this basket |
||
| 48 | * |
||
| 49 | * @var array |
||
| 50 | */ |
||
| 51 | protected $connectShops = []; |
||
| 52 | |||
| 53 | /** |
||
| 54 | * @var \Shopware\Connect\Struct\CheckResult |
||
| 55 | */ |
||
| 56 | protected $checkResult; |
||
| 57 | |||
| 58 | /** |
||
| 59 | * The original shopware shipping costs |
||
| 60 | * |
||
| 61 | * @var float |
||
| 62 | */ |
||
| 63 | protected $originalShippingCosts = 0; |
||
| 64 | |||
| 65 | /** |
||
| 66 | * Should there be a connect hint in the template |
||
| 67 | * |
||
| 68 | * @var bool |
||
| 69 | */ |
||
| 70 | protected $showCheckoutShopInfo; |
||
| 71 | |||
| 72 | /** |
||
| 73 | * @var \Shopware\Connect\SDK |
||
| 74 | */ |
||
| 75 | protected $sdk; |
||
| 76 | |||
| 77 | /** |
||
| 78 | * @var \Enlight_Components_Db_Adapter_Pdo_Mysql |
||
| 79 | */ |
||
| 80 | protected $database; |
||
| 81 | |||
| 82 | /** |
||
| 83 | * @var Helper |
||
| 84 | */ |
||
| 85 | protected $helper; |
||
| 86 | |||
| 87 | /** |
||
| 88 | * @var \Shopware\Connect\Gateway\PDO |
||
| 89 | */ |
||
| 90 | protected $connectGateway; |
||
| 91 | |||
| 92 | /** |
||
| 93 | * Indicates if the basket has only connect products or not |
||
| 94 | * |
||
| 95 | * @var bool |
||
| 96 | */ |
||
| 97 | protected $onlyConnectProducts = false; |
||
| 98 | |||
| 99 | /** |
||
| 100 | * @param \Enlight_Components_Db_Adapter_Pdo_Mysql $database |
||
| 101 | * @param \Shopware\Connect\SDK $sdk |
||
| 102 | * @param Helper $helper |
||
| 103 | * @param $showCheckoutShopInfo |
||
| 104 | */ |
||
| 105 | public function __construct( |
||
| 106 | \Enlight_Components_Db_Adapter_Pdo_Mysql $database, |
||
| 107 | SDK $sdk, |
||
| 108 | Helper $helper, |
||
| 109 | PDO $connectGateway, |
||
| 110 | $showCheckoutShopInfo) |
||
| 111 | { |
||
| 112 | $this->database = $database; |
||
| 113 | $this->sdk = $sdk; |
||
| 114 | $this->helper = $helper; |
||
| 115 | $this->connectGateway = $connectGateway; |
||
| 116 | $this->showCheckoutShopInfo = $showCheckoutShopInfo; |
||
| 117 | } |
||
| 118 | |||
| 119 | /** |
||
| 120 | * Prepare the basket for connect |
||
| 121 | * |
||
| 122 | * @return void |
||
| 123 | */ |
||
| 124 | public function prepareBasketForConnect() |
||
| 125 | { |
||
| 126 | $this->buildProductsArray(); |
||
| 127 | $this->buildShopsArray(); |
||
| 128 | } |
||
| 129 | |||
| 130 | /** |
||
| 131 | * Build array of connect products. This will remove connect products from the 'content' array |
||
| 132 | */ |
||
| 133 | protected function buildProductsArray() |
||
| 134 | { |
||
| 135 | $this->connectProducts = []; |
||
| 136 | $this->connectContent = []; |
||
| 137 | |||
| 138 | $this->basket['contentOrg'] = $this->basket['content']; |
||
| 139 | |||
| 140 | foreach ($this->basket['content'] as $key => &$row) { |
||
| 141 | if (!empty($row['mode'])) { |
||
| 142 | continue; |
||
| 143 | } |
||
| 144 | |||
| 145 | $articleDetailId = $row['additional_details']['articleDetailsID']; |
||
| 146 | if ($this->helper->isRemoteArticleDetailDBAL($articleDetailId) === false) { |
||
| 147 | continue; |
||
| 148 | } |
||
| 149 | |||
| 150 | $shopProductId = $this->helper->getShopProductId($articleDetailId); |
||
| 151 | $products = $this->getHelper()->getRemoteProducts([$shopProductId->sourceId], $shopProductId->shopId); |
||
| 152 | if (empty($products)) { |
||
| 153 | continue; |
||
| 154 | } |
||
| 155 | $product = reset($products); |
||
| 156 | if ($product === null || $product->shopId === null) { |
||
| 157 | continue; |
||
| 158 | } |
||
| 159 | $row['connectShopId'] = $product->shopId; |
||
| 160 | $this->connectProducts[$product->shopId][$product->sourceId] = $product; |
||
| 161 | $this->connectContent[$product->shopId][$product->sourceId] = $row; |
||
| 162 | |||
| 163 | //if($actionName == 'cart') { |
||
|
|
|||
| 164 | unset($this->basket['content'][$key]); |
||
| 165 | //} |
||
| 166 | } |
||
| 167 | } |
||
| 168 | |||
| 169 | /** |
||
| 170 | * Build array of connect remote shops |
||
| 171 | */ |
||
| 172 | protected function buildShopsArray() |
||
| 173 | { |
||
| 174 | $this->connectShops = []; |
||
| 175 | |||
| 176 | $this->basket['content'] = array_values($this->basket['content']); |
||
| 177 | foreach ($this->connectContent as $shopId => $items) { |
||
| 178 | $this->connectShops[$shopId] = $this->getSdk()->getShop($shopId); |
||
| 179 | } |
||
| 180 | } |
||
| 181 | |||
| 182 | /** |
||
| 183 | * Returns the quantity of a given product in the sw basket |
||
| 184 | * |
||
| 185 | * @param \Shopware\Connect\Struct\Product $product |
||
| 186 | * @return mixed |
||
| 187 | */ |
||
| 188 | public function getQuantityForProduct(Product $product) |
||
| 200 | |||
| 201 | /** |
||
| 202 | * This method will check, if any *real* products from the local shop are in the basket. If this is not the |
||
| 203 | * case, this method will: |
||
| 204 | * |
||
| 205 | * - set the first connect shop as content of the default basket ($basket['content']) |
||
| 206 | * - remove any surcharges, vouchers and discount from the original basket(!) |
||
| 207 | * |
||
| 208 | * @return bool|mixed |
||
| 209 | */ |
||
| 210 | public function fixBasket() |
||
| 245 | |||
| 246 | /** |
||
| 247 | * Verifies that some of suppliers as shippingCosts type |
||
| 248 | * different from "remote". Then endcustomer must pay |
||
| 249 | * merchant shipping costs |
||
| 250 | * |
||
| 251 | * @param array $connectContent |
||
| 252 | * @return bool |
||
| 253 | */ |
||
| 254 | private function customerHasToPayLocalShipping(array $connectContent) |
||
| 272 | |||
| 273 | /** |
||
| 274 | * Remove shipping costs from given basket |
||
| 275 | * |
||
| 276 | * @param array $basket |
||
| 277 | * @return array |
||
| 278 | */ |
||
| 279 | private function removeDefaultShipping(array $basket) |
||
| 305 | |||
| 306 | /** |
||
| 307 | * Removes non-connect products from the database and fixes the basket variables |
||
| 308 | */ |
||
| 309 | protected function removeNonProductsFromBasket() |
||
| 374 | |||
| 375 | /** |
||
| 376 | * @todo: This function is basically a copy of the same function in Controllers/Frontend/Checkout. |
||
| 377 | * As that function cannot be called, I copied it for the time being - this should be refactored |
||
| 378 | * |
||
| 379 | * @param $basket array returned from this->getBasket |
||
| 380 | * @return array |
||
| 381 | */ |
||
| 382 | public function getTaxRates($basket) |
||
| 443 | |||
| 444 | /** |
||
| 445 | * Returns an array of tax positions in the same way, as shopware does in the sTaxRates. |
||
| 446 | * This will only take connect products returned by getConnectContent() into account, |
||
| 447 | * so that connect positions moved into basket['content'] earlier are not calculated twice. |
||
| 448 | * |
||
| 449 | * @return array |
||
| 450 | */ |
||
| 451 | public function getConnectTaxRates() |
||
| 472 | |||
| 473 | /** |
||
| 474 | * Will merge/add various arrays of "sTaxRates" like arrays into one array |
||
| 475 | * |
||
| 476 | * @param array $taxRates |
||
| 477 | * @return array |
||
| 478 | */ |
||
| 479 | public function getMergedTaxRates(array $taxRates) |
||
| 494 | |||
| 495 | /** |
||
| 496 | * Increase the basket's shipping costs and amount by the total value of connect shipping costs |
||
| 497 | * |
||
| 498 | * @param \Shopware\Connect\Struct\CheckResult $checkResult |
||
| 499 | */ |
||
| 500 | public function recalculate(CheckResult $checkResult) |
||
| 571 | |||
| 572 | /** |
||
| 573 | * Returns the tax rate of the shipping costs and also sets the the net shipping cost amount(!) |
||
| 574 | */ |
||
| 575 | public function getShippingCostsTaxRates() |
||
| 586 | |||
| 587 | /** |
||
| 588 | * Get the highest tax rate from basket - currently only this is supported by SW |
||
| 589 | * |
||
| 590 | * @return int |
||
| 591 | */ |
||
| 592 | public function getMaxTaxRate() |
||
| 611 | |||
| 612 | /** |
||
| 613 | * Return array of variables which need to be available in the default template |
||
| 614 | * |
||
| 615 | * @return array |
||
| 616 | */ |
||
| 617 | public function getDefaultTemplateVariables() |
||
| 627 | |||
| 628 | /** |
||
| 629 | * Return array of connect specific template variables |
||
| 630 | * |
||
| 631 | * @param $connectMessages array Messages to show |
||
| 632 | * @return array |
||
| 633 | */ |
||
| 634 | public function getConnectTemplateVariables(array $connectMessages) |
||
| 664 | |||
| 665 | /** |
||
| 666 | * Modifies a given OrderVariables ArrayObject |
||
| 667 | * |
||
| 668 | * @param $variables \ArrayObject |
||
| 669 | * @return \ArrayObject |
||
| 670 | */ |
||
| 671 | public function getOrderVariablesForSession($variables) |
||
| 689 | |||
| 690 | /** |
||
| 691 | * Find all percentaged vouchers for a given individual code |
||
| 692 | * |
||
| 693 | * @param $voucherCode |
||
| 694 | * @return mixed |
||
| 695 | */ |
||
| 696 | View Code Duplication | public function findPercentagedIndividualVouchers($voucherCode) |
|
| 708 | |||
| 709 | /** |
||
| 710 | * Find all vouchers matching the code |
||
| 711 | * |
||
| 712 | * @param $voucherCode |
||
| 713 | * @return mixed |
||
| 714 | */ |
||
| 715 | View Code Duplication | public function findPercentagedVouchers($voucherCode) |
|
| 727 | |||
| 728 | /** |
||
| 729 | * @return \Shopware\Connect\SDk |
||
| 730 | */ |
||
| 731 | public function getSdk() |
||
| 735 | |||
| 736 | /** |
||
| 737 | * @return Helper |
||
| 738 | */ |
||
| 739 | public function getHelper() |
||
| 743 | |||
| 744 | /** |
||
| 745 | * @param mixed $basket |
||
| 746 | */ |
||
| 747 | public function setBasket($basket) |
||
| 752 | |||
| 753 | /** |
||
| 754 | * @return mixed |
||
| 755 | */ |
||
| 756 | public function getBasket() |
||
| 760 | |||
| 761 | /** |
||
| 762 | * @param array $connectContent |
||
| 763 | */ |
||
| 764 | public function setConnectContent($connectContent) |
||
| 768 | |||
| 769 | /** |
||
| 770 | * @return array |
||
| 771 | */ |
||
| 772 | public function getConnectContent() |
||
| 776 | |||
| 777 | /** |
||
| 778 | * @param array $connectProducts |
||
| 779 | */ |
||
| 780 | public function setConnectProducts($connectProducts) |
||
| 784 | |||
| 785 | /** |
||
| 786 | * @return array |
||
| 787 | */ |
||
| 788 | public function getConnectProducts() |
||
| 792 | |||
| 793 | /** |
||
| 794 | * @param array $connectShops |
||
| 795 | */ |
||
| 796 | public function setConnectShops($connectShops) |
||
| 800 | |||
| 801 | /** |
||
| 802 | * @return array |
||
| 803 | */ |
||
| 804 | public function getConnectShops() |
||
| 808 | |||
| 809 | /** |
||
| 810 | * @return array |
||
| 811 | */ |
||
| 812 | public function getConnectGrossShippingCosts() |
||
| 829 | |||
| 830 | /** |
||
| 831 | * @param mixed $originalShippingCosts |
||
| 832 | */ |
||
| 833 | public function setOriginalShippingCosts($originalShippingCosts) |
||
| 837 | |||
| 838 | /** |
||
| 839 | * @return mixed |
||
| 840 | */ |
||
| 841 | public function getOriginalShippingCosts() |
||
| 845 | |||
| 846 | /** |
||
| 847 | * @param \Enlight_Components_Db_Adapter_Pdo_Mysql $database |
||
| 848 | */ |
||
| 849 | public function setDatabase($database) |
||
| 853 | |||
| 854 | /** |
||
| 855 | * @return \Enlight_Components_Db_Adapter_Pdo_Mysql |
||
| 856 | */ |
||
| 857 | public function getDatabase() |
||
| 861 | |||
| 862 | /** |
||
| 863 | * Returns "Gross price displayed in frontend" value |
||
| 864 | * @return bool |
||
| 865 | */ |
||
| 866 | protected function hasTax() |
||
| 878 | |||
| 879 | /** |
||
| 880 | * @return \Shopware\Connect\Struct\CheckResult |
||
| 881 | */ |
||
| 882 | public function getCheckResult() |
||
| 886 | |||
| 887 | /** |
||
| 888 | * @param \Shopware\Connect\Struct\CheckResult $checkResult |
||
| 889 | */ |
||
| 890 | public function setCheckResult(CheckResult $checkResult) |
||
| 894 | } |
||
| 895 |
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.