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 ProductFromShop 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 ProductFromShop, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 37 | class ProductFromShop implements ProductFromShopBase |
||
| 38 | { |
||
| 39 | /** |
||
| 40 | * @var Helper |
||
| 41 | */ |
||
| 42 | private $helper; |
||
| 43 | |||
| 44 | /** |
||
| 45 | * @var ModelManager |
||
| 46 | */ |
||
| 47 | private $manager; |
||
| 48 | |||
| 49 | /** |
||
| 50 | * @var \Shopware\Connect\Gateway |
||
| 51 | */ |
||
| 52 | private $gateway; |
||
| 53 | |||
| 54 | /** |
||
| 55 | * @var Logger |
||
| 56 | */ |
||
| 57 | private $logger; |
||
| 58 | |||
| 59 | /** |
||
| 60 | * @var Enlight_Event_EventManager |
||
| 61 | */ |
||
| 62 | private $eventManager; |
||
| 63 | |||
| 64 | /** |
||
| 65 | * @param Helper $helper |
||
| 66 | * @param ModelManager $manager |
||
| 67 | * @param Gateway $gateway |
||
| 68 | * @param Logger $logger |
||
| 69 | * @param Enlight_Event_EventManager $eventManager |
||
| 70 | */ |
||
| 71 | public function __construct( |
||
| 72 | Helper $helper, |
||
| 73 | ModelManager $manager, |
||
| 74 | Gateway $gateway, |
||
| 75 | Logger $logger, |
||
| 76 | Enlight_Event_EventManager $eventManager |
||
| 77 | ) { |
||
| 78 | $this->helper = $helper; |
||
| 79 | $this->manager = $manager; |
||
| 80 | $this->gateway = $gateway; |
||
| 81 | $this->logger = $logger; |
||
| 82 | $this->eventManager = $eventManager; |
||
| 83 | } |
||
| 84 | |||
| 85 | /** |
||
| 86 | * Get product data |
||
| 87 | * |
||
| 88 | * Get product data for all the source IDs specified in the given string |
||
| 89 | * array. |
||
| 90 | * |
||
| 91 | * @param string[] $sourceIds |
||
| 92 | * @return Product[] |
||
| 93 | */ |
||
| 94 | public function getProducts(array $sourceIds) |
||
| 95 | { |
||
| 96 | return $this->helper->getLocalProduct($sourceIds); |
||
| 97 | } |
||
| 98 | |||
| 99 | /** |
||
| 100 | * Get all IDs of all exported products |
||
| 101 | * |
||
| 102 | * @throws \BadMethodCallException |
||
| 103 | * @return string[] |
||
| 104 | */ |
||
| 105 | public function getExportedProductIDs() |
||
| 106 | { |
||
| 107 | throw new \BadMethodCallException('Not implemented'); |
||
| 108 | } |
||
| 109 | |||
| 110 | /** |
||
| 111 | * Reserve a product in shop for purchase |
||
| 112 | * |
||
| 113 | * @param Order $order |
||
| 114 | * @throws \Exception Abort reservation by throwing an exception here. |
||
| 115 | * @return void |
||
| 116 | */ |
||
| 117 | public function reserve(Order $order) |
||
| 118 | { |
||
| 119 | $this->eventManager->notify( |
||
| 120 | 'Connect_Supplier_Reservation_Before', |
||
| 121 | [ |
||
| 122 | 'subject' => $this, |
||
| 123 | 'order' => $order |
||
| 124 | ] |
||
| 125 | ); |
||
| 126 | } |
||
| 127 | |||
| 128 | /** |
||
| 129 | * Create order in shopware |
||
| 130 | * Wraps the actual order process into a transaction |
||
| 131 | * |
||
| 132 | * |
||
| 133 | * @param Order $order |
||
| 134 | * @throws \Exception Abort buy by throwing an exception, |
||
| 135 | * but only in very important cases. |
||
| 136 | * Do validation in {@see reserve} instead. |
||
| 137 | * @return string |
||
| 138 | */ |
||
| 139 | public function buy(Order $order) |
||
| 140 | { |
||
| 141 | $this->manager->beginTransaction(); |
||
| 142 | try { |
||
| 143 | $order = $this->eventManager->filter('Connect_Components_ProductFromShop_Buy_OrderFilter', $order); |
||
| 144 | |||
| 145 | $this->validateBilling($order->billingAddress); |
||
| 146 | $orderNumber = $this->doBuy($order); |
||
| 147 | |||
| 148 | $this->manager->commit(); |
||
| 149 | } catch (\Exception $e) { |
||
| 150 | $this->manager->rollback(); |
||
| 151 | throw $e; |
||
| 152 | } |
||
| 153 | |||
| 154 | return $orderNumber; |
||
| 155 | } |
||
| 156 | |||
| 157 | /** |
||
| 158 | * Actually creates the remote order in shopware. |
||
| 159 | * |
||
| 160 | * @param Order $order |
||
| 161 | * @return string |
||
| 162 | */ |
||
| 163 | public function doBuy(Order $order) |
||
| 164 | { |
||
| 165 | $this->manager->clear(); |
||
| 166 | |||
| 167 | $detailStatus = $this->manager->find('Shopware\Models\Order\DetailStatus', 0); |
||
| 168 | $status = $this->manager->find('Shopware\Models\Order\Status', 0); |
||
| 169 | $shop = $this->manager->find('Shopware\Models\Shop\Shop', 1); |
||
| 170 | $number = 'SC-' . $order->orderShop . '-' . $order->localOrderId; |
||
| 171 | |||
| 172 | $repository = $this->manager->getRepository('Shopware\Models\Payment\Payment'); |
||
| 173 | $payment = $repository->findOneBy([ |
||
| 174 | 'name' => 'invoice', |
||
| 175 | ]); |
||
| 176 | |||
| 177 | // todo: Create the OrderModel without previous plain SQL |
||
| 178 | //$model = new OrderModel\Order(); |
||
|
|
|||
| 179 | $sql = 'INSERT INTO `s_order` (`ordernumber`, `cleared`) VALUES (?, 17);'; |
||
| 180 | Shopware()->Db()->query($sql, [$number]); |
||
| 181 | $modelId = Shopware()->Db()->lastInsertId(); |
||
| 182 | /** @var $model \Shopware\Models\Order\Order */ |
||
| 183 | $model = $this->manager->find('Shopware\Models\Order\Order', $modelId); |
||
| 184 | |||
| 185 | $attribute = new \Shopware\Models\Attribute\Order; |
||
| 186 | $attribute->setConnectOrderId($order->localOrderId); |
||
| 187 | $attribute->setConnectShopId($order->orderShop); |
||
| 188 | $model->setAttribute($attribute); |
||
| 189 | |||
| 190 | $model->fromArray([ |
||
| 191 | 'number' => $number, |
||
| 192 | 'invoiceShipping' => $order->grossShippingCosts, |
||
| 193 | 'invoiceShippingNet' => $order->shippingCosts, |
||
| 194 | 'currencyFactor' => 1, |
||
| 195 | 'orderStatus' => $status, |
||
| 196 | 'shop' => $shop, |
||
| 197 | 'languageSubShop' => $shop, |
||
| 198 | 'payment' => $payment, |
||
| 199 | 'currency' => 'EUR', |
||
| 200 | 'orderTime' => 'now' |
||
| 201 | ]); |
||
| 202 | $items = []; |
||
| 203 | $connectAttributeRepository = $this->manager->getRepository('Shopware\CustomModels\Connect\Attribute'); |
||
| 204 | |||
| 205 | /** @var \Shopware\Connect\Struct\OrderItem $orderItem */ |
||
| 206 | foreach ($order->products as $orderItem) { |
||
| 207 | $product = $orderItem->product; |
||
| 208 | /** @var \Shopware\CustomModels\Connect\Attribute $connectAttribute */ |
||
| 209 | $connectAttribute = $connectAttributeRepository->findOneBy([ |
||
| 210 | 'sourceId' => $product->sourceId, |
||
| 211 | 'shopId' => null, |
||
| 212 | ]); |
||
| 213 | if (!$connectAttribute) { |
||
| 214 | $this->logger->write( |
||
| 215 | true, |
||
| 216 | sprintf('Detail with sourceId: %s does not exist', $product->sourceId), |
||
| 217 | null, |
||
| 218 | true |
||
| 219 | ); |
||
| 220 | continue; |
||
| 221 | } |
||
| 222 | |||
| 223 | /** @var $detail \Shopware\Models\Article\Detail */ |
||
| 224 | $detail = $connectAttribute->getArticleDetail(); |
||
| 225 | /** @var $productModel \Shopware\Models\Article\Article */ |
||
| 226 | $productModel = $detail->getArticle(); |
||
| 227 | $item = new OrderModel\Detail(); |
||
| 228 | $item->fromArray([ |
||
| 229 | 'articleId' => $productModel->getId(), |
||
| 230 | 'quantity' => $orderItem->count, |
||
| 231 | 'orderId' => $model->getId(), |
||
| 232 | 'number' => $model->getNumber(), |
||
| 233 | 'articleNumber' => $detail->getNumber(), |
||
| 234 | 'articleName' => $product->title, |
||
| 235 | 'price' => $this->calculatePrice($product), |
||
| 236 | 'taxRate' => $product->vat * 100, |
||
| 237 | 'status' => $detailStatus, |
||
| 238 | 'attribute' => new OrderDetailAttributeModel() |
||
| 239 | ]); |
||
| 240 | $items[] = $item; |
||
| 241 | } |
||
| 242 | $model->setDetails($items); |
||
| 243 | |||
| 244 | $email = $order->billingAddress->email; |
||
| 245 | |||
| 246 | $password = Random::getAlphanumericString(30); |
||
| 247 | |||
| 248 | $repository = $this->manager->getRepository('Shopware\Models\Customer\Customer'); |
||
| 249 | $customer = $repository->findOneBy([ |
||
| 250 | 'email' => $email |
||
| 251 | ]); |
||
| 252 | if ($customer === null) { |
||
| 253 | $customer = new CustomerModel\Customer(); |
||
| 254 | $customer->fromArray([ |
||
| 255 | 'active' => true, |
||
| 256 | 'email' => $email, |
||
| 257 | 'password' => $password, |
||
| 258 | 'accountMode' => 1, |
||
| 259 | 'shop' => $shop, |
||
| 260 | 'paymentId' => $payment->getId(), |
||
| 261 | ]); |
||
| 262 | } |
||
| 263 | if ($customer->getBilling() === null) { |
||
| 264 | $billing = new CustomerModel\Billing(); |
||
| 265 | $customer->setBilling($billing); |
||
| 266 | } else { |
||
| 267 | $billing = $customer->getBilling(); |
||
| 268 | } |
||
| 269 | |||
| 270 | $billing->fromArray($this->getAddressData( |
||
| 271 | $order->billingAddress |
||
| 272 | )); |
||
| 273 | $this->manager->persist($customer); |
||
| 274 | |||
| 275 | $model->setCustomer($customer); |
||
| 276 | |||
| 277 | $billing = new OrderModel\Billing(); |
||
| 278 | $billing->setCustomer($customer); |
||
| 279 | $billing->fromArray($this->getAddressData( |
||
| 280 | $order->billingAddress |
||
| 281 | )); |
||
| 282 | $model->setBilling($billing); |
||
| 283 | |||
| 284 | $shipping = new OrderModel\Shipping(); |
||
| 285 | $shipping->setCustomer($customer); |
||
| 286 | $shipping->fromArray($this->getAddressData( |
||
| 287 | $order->deliveryAddress |
||
| 288 | )); |
||
| 289 | $model->setShipping($shipping); |
||
| 290 | |||
| 291 | $model->calculateInvoiceAmount(); |
||
| 292 | |||
| 293 | $dispatchRepository = $this->manager->getRepository('Shopware\Models\Dispatch\Dispatch'); |
||
| 294 | $dispatch = $dispatchRepository->findOneBy([ |
||
| 295 | 'name' => $order->shipping->service |
||
| 296 | ]); |
||
| 297 | if ($dispatch) { |
||
| 298 | $model->setDispatch($dispatch); |
||
| 299 | } |
||
| 300 | |||
| 301 | $this->eventManager->notify( |
||
| 302 | 'Connect_Supplier_Buy_Before', |
||
| 303 | [ |
||
| 304 | 'subject' => $this, |
||
| 305 | 'order' => $order |
||
| 306 | ] |
||
| 307 | ); |
||
| 308 | |||
| 309 | $this->manager->flush(); |
||
| 310 | |||
| 311 | return $model->getNumber(); |
||
| 312 | } |
||
| 313 | |||
| 314 | /** |
||
| 315 | * Calculate the price (including VAT) that the from shop needs to pay. |
||
| 316 | * |
||
| 317 | * This is most likely NOT the price the customer itself has to pay. |
||
| 318 | * |
||
| 319 | * @return float |
||
| 320 | */ |
||
| 321 | private function calculatePrice($product) |
||
| 322 | { |
||
| 323 | return $product->purchasePrice * ($product->vat + 1); |
||
| 324 | } |
||
| 325 | |||
| 326 | /** |
||
| 327 | * @param Address $address |
||
| 328 | * @return array |
||
| 329 | */ |
||
| 330 | private function getAddressData(Address $address) |
||
| 331 | { |
||
| 332 | $repository = 'Shopware\Models\Country\Country'; |
||
| 333 | $repository = $this->manager->getRepository($repository); |
||
| 334 | /** @var $country \Shopware\Models\Country\Country */ |
||
| 335 | $country = $repository->findOneBy([ |
||
| 336 | 'iso3' => $address->country |
||
| 337 | ]); |
||
| 338 | |||
| 339 | return [ |
||
| 340 | 'company' => $address->company ?: '', |
||
| 341 | 'department' => $address->department ?: '', |
||
| 342 | 'additionalAddressLine1' => $address->additionalAddressLine1 ?: '', |
||
| 343 | 'additionalAddressLine2' => $address->additionalAddressLine2 ?: '', |
||
| 344 | 'salutation' => 'mr', |
||
| 345 | 'lastName' => $address->surName, |
||
| 346 | 'firstName' => $address->firstName, |
||
| 347 | 'city' => $address->city, |
||
| 348 | 'zipCode' => $address->zip, |
||
| 349 | 'street' => $address->street, |
||
| 350 | 'streetNumber' => $address->streetNumber, |
||
| 351 | 'phone' => $address->phone, |
||
| 352 | 'country' => $country |
||
| 353 | ]; |
||
| 354 | } |
||
| 355 | |||
| 356 | public function updatePaymentStatus(PaymentStatus $status) |
||
| 408 | |||
| 409 | public function calculateShippingCosts(Order $order) |
||
| 410 | { |
||
| 411 | View Code Duplication | if (!$order->deliveryAddress) { |
|
| 412 | return new Shipping([ |
||
| 413 | 'isShippable' => false, |
||
| 414 | 'messages' => [ |
||
| 415 | new Message([ |
||
| 416 | 'message' => 'delivery_address_empty' |
||
| 552 | |||
| 553 | /** |
||
| 554 | * Perform sync changes to fromShop |
||
| 555 | * |
||
| 556 | * @param string $since |
||
| 557 | * @param \Shopware\Connect\Struct\Change[] $changes |
||
| 558 | * @return void |
||
| 559 | */ |
||
| 560 | public function onPerformSync($since, array $changes) |
||
| 634 | |||
| 635 | View Code Duplication | private function markStreamsAsNotExported() |
|
| 667 | |||
| 668 | View Code Duplication | private function markStreamsAsSynced() |
|
| 700 | |||
| 701 | /** |
||
| 702 | * @param Address $address |
||
| 703 | */ |
||
| 704 | private function validateBilling(Address $address) |
||
| 730 | } |
||
| 731 |
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.