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 | 59 | public function __construct( |
|
| 244 | |||
| 245 | /** |
||
| 246 | * セッションにセットされた受注情報を取得 |
||
| 247 | * |
||
| 248 | * @param null $status |
||
| 249 | * |
||
| 250 | * @return null|object |
||
| 251 | */ |
||
| 252 | 51 | 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 | 6 | * @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.