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 ShoppingService 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 ShoppingService, and based on these observations, apply Extract Interface, too.
| 1 | <?php | ||
| 57 | class ShoppingService | ||
| 58 | { | ||
| 59 | /** | ||
| 60 | * @var MailTemplateRepository | ||
| 61 | */ | ||
| 62 | protected $mailTemplateRepository; | ||
| 63 | |||
| 64 | /** | ||
| 65 | * @var MailService | ||
| 66 | */ | ||
| 67 | protected $mailService; | ||
| 68 | |||
| 69 | /** | ||
| 70 | * @var EventDispatcher | ||
| 71 | */ | ||
| 72 | protected $eventDispatcher; | ||
| 73 | |||
| 74 | /** | ||
| 75 | * @var FormFactory | ||
| 76 | */ | ||
| 77 | protected $formFactory; | ||
| 78 | |||
| 79 | /** | ||
| 80 | * @var DeliveryFeeRepository | ||
| 81 | */ | ||
| 82 | protected $deliveryFeeRepository; | ||
| 83 | |||
| 84 | /** | ||
| 85 | * @var TaxRuleRepository | ||
| 86 | */ | ||
| 87 | protected $taxRuleRepository; | ||
| 88 | |||
| 89 | /** | ||
| 90 | * @var CustomerAddressRepository | ||
| 91 | */ | ||
| 92 | protected $customerAddressRepository; | ||
| 93 | |||
| 94 | /** | ||
| 95 | * @var DeliveryRepository | ||
| 96 | */ | ||
| 97 | protected $deliveryRepository; | ||
| 98 | |||
| 99 | /** | ||
| 100 | * @var DeliveryTimeRepository | ||
| 101 | */ | ||
| 102 | protected $deliveryTimeRepository; | ||
| 103 | |||
| 104 | /** | ||
| 105 | * @var OrderStatusRepository | ||
| 106 | */ | ||
| 107 | protected $orderStatusRepository; | ||
| 108 | |||
| 109 | /** | ||
| 110 | * @var PaymentRepository | ||
| 111 | */ | ||
| 112 | protected $paymentRepository; | ||
| 113 | |||
| 114 | /** | ||
| 115 | * @var DeviceTypeRepository | ||
| 116 | */ | ||
| 117 | protected $deviceTypeRepository; | ||
| 118 | |||
| 119 | /** | ||
| 120 | * @var EntityManager | ||
| 121 | */ | ||
| 122 | protected $entityManager; | ||
| 123 | |||
| 124 | /** | ||
| 125 | * @var EccubeConfig | ||
| 126 | */ | ||
| 127 | protected $eccubeConfig; | ||
| 128 | |||
| 129 | /** | ||
| 130 | * @var PrefRepository | ||
| 131 | */ | ||
| 132 | protected $prefRepository; | ||
| 133 | |||
| 134 | /** | ||
| 135 | * @var Session | ||
| 136 | */ | ||
| 137 | protected $session; | ||
| 138 | |||
| 139 | /** | ||
| 140 | * @var OrderRepository | ||
| 141 | */ | ||
| 142 | protected $orderRepository; | ||
| 143 | |||
| 144 | /** | ||
| 145 | * @var BaseInfo | ||
| 146 | */ | ||
| 147 | protected $BaseInfo; | ||
| 148 | |||
| 149 | /** | ||
| 150 | * @var \Eccube\Service\CartService | ||
| 151 | */ | ||
| 152 | protected $cartService; | ||
| 153 | |||
| 154 | /** | ||
| 155 | * @var \Eccube\Service\OrderService | ||
| 156 | * | ||
| 157 | * @deprecated | ||
| 158 | */ | ||
| 159 | protected $orderService; | ||
| 160 | |||
| 161 | /** | ||
| 162 | * @var AuthorizationCheckerInterface | ||
| 163 | */ | ||
| 164 | protected $authorizationChecker; | ||
| 165 | |||
| 166 | /** | ||
| 167 | * @var \Mobile_Detect | ||
| 168 | */ | ||
| 169 | protected $mobileDetect; | ||
| 170 | |||
| 171 | /** | ||
| 172 | * ShoppingService constructor. | ||
| 173 | * | ||
| 174 | * @param MailTemplateRepository $mailTemplateRepository | ||
| 175 | * @param MailService $mailService | ||
| 176 | * @param EventDispatcher $eventDispatcher | ||
| 177 | * @param FormFactory $formFactory | ||
| 178 | * @param DeliveryFeeRepository $deliveryFeeRepository | ||
| 179 | * @param TaxRuleRepository $taxRuleRepository | ||
| 180 | * @param CustomerAddressRepository $customerAddressRepository | ||
| 181 | * @param DeliveryRepository $deliveryRepository | ||
| 182 | * @param DeliveryTimeRepository $deliveryTimeRepository | ||
| 183 | * @param OrderStatusRepository $orderStatusRepository | ||
| 184 | * @param PaymentRepository $paymentRepository | ||
| 185 | * @param DeviceTypeRepository $deviceTypeRepository | ||
| 186 | * @param EntityManager $entityManager | ||
| 187 | * @param EccubeConfig $eccubeConfig | ||
| 188 | * @param PrefRepository $prefRepository | ||
| 189 | * @param Session $session | ||
| 190 | * @param OrderRepository $orderRepository | ||
| 191 | * @param CartService $cartService | ||
| 192 | * @param OrderService $orderService | ||
| 193 | * @param BaseInfoRepository $baseInfoRepository | ||
| 194 | * @param AuthorizationCheckerInterface $authorizationChecker | ||
| 195 | * @param \Mobile_Detect $mobileDetect | ||
| 196 | */ | ||
| 197 | public function __construct( | ||
| 244 | |||
| 245 | /** | ||
| 246 | * セッションにセットされた受注情報を取得 | ||
| 247 | * | ||
| 248 | * @param null $status | ||
| 249 | * | ||
| 250 | * @return null|object | ||
| 251 | */ | ||
| 252 | public function getOrder($status = null) | ||
| 274 | |||
| 275 | /** | ||
| 276 | * 非会員情報を取得 | ||
| 277 | * | ||
| 278 | * @param string $sesisonKey | ||
| 279 | * | ||
| 280 | * @return $Customer|null | ||
| 281 | */ | ||
| 282 | public function getNonMember($sesisonKey) | ||
| 291 | |||
| 292 | /** | ||
| 293 | * 仮受注情報作成 | ||
| 294 | * | ||
| 295 | * @param $Customer | ||
| 296 | * @param string $preOrderId | ||
| 297 | * | ||
| 298 | * @return Order | ||
| 299 | * | ||
| 300 | * @throws \Doctrine\ORM\NoResultException | ||
| 301 | * @throws \Doctrine\ORM\NonUniqueResultException | ||
| 302 | */ | ||
| 303 | public function registerPreOrder(Customer $Customer, $preOrderId) | ||
| 366 | |||
| 367 | /** | ||
| 368 | * 受注情報を作成 | ||
| 369 | * | ||
| 370 | * @param $Customer | ||
| 371 | * | ||
| 372 | * @return \Eccube\Entity\Order | ||
| 373 | */ | ||
| 374 | public function getNewOrder(Customer $Customer) | ||
| 381 | |||
| 382 | /** | ||
| 383 | * 受注情報を作成 | ||
| 384 | * | ||
| 385 | * @return \Eccube\Entity\Order | ||
| 386 | */ | ||
| 387 | public function newOrder() | ||
| 394 | |||
| 395 | /** | ||
| 396 | * 受注情報を作成 | ||
| 397 | * | ||
| 398 | * @param \Eccube\Entity\Order $Order | ||
| 399 | * @param \Eccube\Entity\Customer|null $Customer | ||
| 400 | * | ||
| 401 | * @return \Eccube\Entity\Order | ||
| 402 | */ | ||
| 403 | public function copyToOrderFromCustomer(Order $Order, Customer $Customer = null) | ||
| 430 | |||
| 431 | /** | ||
| 432 | * 配送業者情報を取得 | ||
| 433 | * | ||
| 434 | * @return array | ||
| 435 | */ | ||
| 436 | public function getDeliveriesCart() | ||
| 443 | |||
| 444 | /** | ||
| 445 | * 配送業者情報を取得 | ||
| 446 | * | ||
| 447 | * @param Order $Order | ||
| 448 | * | ||
| 449 | * @return array | ||
| 450 | */ | ||
| 451 | public function getDeliveriesOrder(Order $Order) | ||
| 458 | |||
| 459 | /** | ||
| 460 | * 配送業者情報を取得 | ||
| 461 | * | ||
| 462 | * @param $saleTypes | ||
| 463 | * | ||
| 464 | * @return array | ||
| 465 | */ | ||
| 466 | public function getDeliveries($saleTypes) | ||
| 481 | |||
| 482 | /** | ||
| 483 | * お届け先情報を作成 | ||
| 484 | * | ||
| 485 | * @param Order $Order | ||
| 486 | * @param Customer $Customer | ||
| 487 | * @param $deliveries | ||
| 488 | * | ||
| 489 | * @return Order | ||
| 490 | */ | ||
| 491 | public function getNewShipping(Order $Order, Customer $Customer, $deliveries) | ||
| 516 | |||
| 517 | /** | ||
| 518 | * お届け先情報を作成 | ||
| 519 | * | ||
| 520 | * @param \Eccube\Entity\Shipping $Shipping | ||
| 521 | * @param \Eccube\Entity\Customer|null $Customer | ||
| 522 | * | ||
| 523 | * @return \Eccube\Entity\Shipping | ||
| 524 | */ | ||
| 525 | public function copyToShippingFromCustomer(Shipping $Shipping, Customer $Customer = null) | ||
| 565 | |||
| 566 | /** | ||
| 567 | * 受注明細情報、配送商品情報を作成 | ||
| 568 | * | ||
| 569 | * @param Order $Order | ||
| 570 | * | ||
| 571 | * @return Order | ||
| 572 | */ | ||
| 573 | public function getNewDetails(Order $Order) | ||
| 591 | |||
| 592 | /** | ||
| 593 | * 配送商品情報を作成 | ||
| 594 | * | ||
| 595 | * @param Order $Order | ||
| 596 | * @param Product $Product | ||
| 597 | * @param ProductClass $ProductClass | ||
| 598 | * @param $quantity | ||
| 599 | * | ||
| 600 | * @return \Eccube\Entity\OrderItem | ||
| 601 | */ | ||
| 602 | public function getNewOrderItem(Order $Order, Product $Product, ProductClass $ProductClass, $quantity) | ||
| 659 | |||
| 660 | /** | ||
| 661 | * お届け先ごとの送料合計を取得 | ||
| 662 | * | ||
| 663 | * @param $shippings | ||
| 664 | * | ||
| 665 | * @return int | ||
| 666 | */ | ||
| 667 | public function getShippingDeliveryFeeTotal($shippings) | ||
| 676 | |||
| 677 | /** | ||
| 678 | * 商品ごとの配送料を取得 | ||
| 679 | * | ||
| 680 | * @param Shipping $Shipping | ||
| 681 | * | ||
| 682 | * @return int | ||
| 683 | */ | ||
| 684 | public function getProductDeliveryFee(Shipping $Shipping) | ||
| 695 | |||
| 696 | /** | ||
| 697 | * 配送料金の設定 | ||
| 698 | * | ||
| 699 | * @param Shipping $Shipping | ||
| 700 | * @param Delivery|null $Delivery | ||
| 701 | */ | ||
| 702 | public function setShippingDeliveryFee(Shipping $Shipping, Delivery $Delivery = null) | ||
| 723 | |||
| 724 | /** | ||
| 725 | * 配送料無料条件(合計金額)の条件を満たしていれば配送料金を0に設定 | ||
| 726 | * | ||
| 727 | * @param Order $Order | ||
| 728 | */ | ||
| 729 | View Code Duplication | public function setDeliveryFreeAmount(Order $Order) | |
| 745 | |||
| 746 | /** | ||
| 747 | * 配送料無料条件(合計数量)の条件を満たしていれば配送料金を0に設定 | ||
| 748 | * | ||
| 749 | * @param Order $Order | ||
| 750 | */ | ||
| 751 | View Code Duplication | public function setDeliveryFreeQuantity(Order $Order) | |
| 767 | |||
| 768 | /** | ||
| 769 | * 受注情報、お届け先情報の更新 | ||
| 770 | * | ||
| 771 | * @param Order $Order 受注情報 | ||
| 772 | * @param $data フォームデータ | ||
| 773 | * | ||
| 774 | * @deprecated since 3.0.5, to be removed in 3.1 | ||
| 775 | */ | ||
| 776 | public function setOrderUpdate(Order $Order, $data) | ||
| 809 | |||
| 810 | /** | ||
| 811 | * 受注情報の更新 | ||
| 812 | * | ||
| 813 | * @param Order $Order 受注情報 | ||
| 814 | */ | ||
| 815 | public function setOrderUpdateData(Order $Order) | ||
| 822 | |||
| 823 | /** | ||
| 824 | * 会員情報の更新 | ||
| 825 | * | ||
| 826 | * @param Order $Order 受注情報 | ||
| 827 | * @param Customer $user ログインユーザ | ||
| 828 | */ | ||
| 829 | public function setCustomerUpdate(Order $Order, Customer $user) | ||
| 842 | |||
| 843 | /** | ||
| 844 | * 支払方法選択の表示設定 | ||
| 845 | * | ||
| 846 | * @param $payments 支払選択肢情報 | ||
| 847 | * @param $subTotal 小計 | ||
| 848 | * | ||
| 849 | * @return array | ||
| 850 | */ | ||
| 851 | public function getPayments($payments, $subTotal) | ||
| 868 | |||
| 869 | /** | ||
| 870 | * お届け日を取得 | ||
| 871 | * | ||
| 872 | * @param Order $Order | ||
| 873 | * | ||
| 874 | * @return array | ||
| 875 | */ | ||
| 876 | public function getFormDeliveryDurations(Order $Order) | ||
| 922 | |||
| 923 | /** | ||
| 924 | * 支払方法を取得 | ||
| 925 | * | ||
| 926 | * @param $deliveries | ||
| 927 | * @param Order $Order | ||
| 928 | * | ||
| 929 | * @return array | ||
| 930 | */ | ||
| 931 | public function getFormPayments($deliveries, Order $Order) | ||
| 946 | |||
| 947 | /** | ||
| 948 | * お届け先ごとにFormを作成 | ||
| 949 | * | ||
| 950 | * @param Order $Order | ||
| 951 | * | ||
| 952 | * @return \Symfony\Component\Form\Form | ||
| 953 | * | ||
| 954 | * @deprecated since 3.0, to be removed in 3.1 | ||
| 955 | */ | ||
| 956 | View Code Duplication | public function getShippingForm(Order $Order) | |
| 981 | |||
| 982 | /** | ||
| 983 | * お届け先ごとにFormBuilderを作成 | ||
| 984 | * | ||
| 985 | * @param Order $Order | ||
| 986 | * | ||
| 987 | * @return \Symfony\Component\Form\FormBuilderInterface | ||
| 988 | * | ||
| 989 | * @deprecated 利用している箇所なし | ||
| 990 | */ | ||
| 991 | View Code Duplication | public function getShippingFormBuilder(Order $Order) | |
| 1014 | |||
| 1015 | /** | ||
| 1016 | * 配送料の合計金額を計算 | ||
| 1017 | * | ||
| 1018 | * @param Order $Order | ||
| 1019 | * | ||
| 1020 | * @return Order | ||
| 1021 | */ | ||
| 1022 | public function calculateDeliveryFee(Order $Order) | ||
| 1039 | |||
| 1040 | /** | ||
| 1041 | * 購入処理を行う | ||
| 1042 | * | ||
| 1043 | * @param Order $Order | ||
| 1044 | * | ||
| 1045 | * @deprecated PurchaseFlow::purchase() を使用してください | ||
| 1046 | */ | ||
| 1047 | public function processPurchase(Order $Order) | ||
| 1057 | |||
| 1058 | /** | ||
| 1059 | * 値引き可能かチェック | ||
| 1060 | * | ||
| 1061 | * @param Order $Order | ||
| 1062 | * @param $discount | ||
| 1063 | * | ||
| 1064 | * @return bool | ||
| 1065 | */ | ||
| 1066 | public function isDiscount(Order $Order, $discount) | ||
| 1074 | |||
| 1075 | /** | ||
| 1076 | * 値引き金額をセット | ||
| 1077 | * | ||
| 1078 | * @param Order $Order | ||
| 1079 | * @param $discount | ||
| 1080 | */ | ||
| 1081 | public function setDiscount(Order $Order, $discount) | ||
| 1085 | |||
| 1086 | /** | ||
| 1087 | * 合計金額を計算 | ||
| 1088 | * | ||
| 1089 | * @param Order $Order | ||
| 1090 | * | ||
| 1091 | * @return Order | ||
| 1092 | */ | ||
| 1093 | public function calculatePrice(Order $Order) | ||
| 1107 | |||
| 1108 | /** | ||
| 1109 | * 受注ステータスをセット | ||
| 1110 | * | ||
| 1111 | * @param Order $Order | ||
| 1112 | * @param $status | ||
| 1113 | * | ||
| 1114 | * @return Order | ||
| 1115 | */ | ||
| 1116 | public function setOrderStatus(Order $Order, $status) | ||
| 1131 | |||
| 1132 | /** | ||
| 1133 | * 受注メール送信を行う | ||
| 1134 | * | ||
| 1135 | * @param Order $Order | ||
| 1136 | * | ||
| 1137 | * @return MailHistory | ||
| 1138 | */ | ||
| 1139 | public function sendOrderMail(Order $Order) | ||
| 1157 | |||
| 1158 | /** | ||
| 1159 | * 受注処理完了通知 | ||
| 1160 | * | ||
| 1161 | * @param Order $Order | ||
| 1162 | */ | ||
| 1163 | public function notifyComplete(Order $Order) | ||
| 1173 | } | ||
| 1174 | 
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 given class or a super-class is assigned to a property that is type hinted more strictly.
Either this assignment is in error or an instanceof check should be added for that assignment.