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 ManagingOrdersContext 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 ManagingOrdersContext, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 30 | final class ManagingOrdersContext implements Context |
||
| 31 | { |
||
| 32 | /** |
||
| 33 | * @var SharedStorageInterface |
||
| 34 | */ |
||
| 35 | private $sharedStorage; |
||
| 36 | |||
| 37 | /** |
||
| 38 | * @var IndexPageInterface |
||
| 39 | */ |
||
| 40 | private $indexPage; |
||
| 41 | |||
| 42 | /** |
||
| 43 | * @var ShowPageInterface |
||
| 44 | */ |
||
| 45 | private $showPage; |
||
| 46 | |||
| 47 | /** |
||
| 48 | * @var NotificationCheckerInterface |
||
| 49 | */ |
||
| 50 | private $notificationChecker; |
||
| 51 | |||
| 52 | /** |
||
| 53 | * @var SecurityServiceInterface |
||
| 54 | */ |
||
| 55 | private $securityService; |
||
| 56 | |||
| 57 | /** |
||
| 58 | * @param SharedStorageInterface $sharedStorage |
||
| 59 | * @param IndexPageInterface $indexPage |
||
| 60 | * @param ShowPageInterface $showPage |
||
| 61 | * @param NotificationCheckerInterface $notificationChecker |
||
| 62 | * @param SecurityServiceInterface $securityService |
||
| 63 | */ |
||
| 64 | View Code Duplication | public function __construct( |
|
| 77 | |||
| 78 | /** |
||
| 79 | * @When I browse orders |
||
| 80 | */ |
||
| 81 | public function iBrowseOrders() |
||
| 85 | |||
| 86 | /** |
||
| 87 | * @When I view the summary of the order :order |
||
| 88 | */ |
||
| 89 | public function iSeeTheOrder(OrderInterface $order) |
||
| 93 | |||
| 94 | /** |
||
| 95 | * @When /^I mark (this order) as a paid$/ |
||
| 96 | */ |
||
| 97 | public function iMarkThisOrderAsAPaid(OrderInterface $order) |
||
| 101 | |||
| 102 | /** |
||
| 103 | * @When specify its tracking code as :trackingCode |
||
| 104 | */ |
||
| 105 | public function specifyItsTrackingCodeAs($trackingCode) |
||
| 109 | |||
| 110 | /** |
||
| 111 | * @Given /^I ship (this order)$/ |
||
| 112 | */ |
||
| 113 | public function iShipThisOrder(OrderInterface $order) |
||
| 117 | |||
| 118 | /** |
||
| 119 | * @Then I should see a single order from customer :customer |
||
| 120 | */ |
||
| 121 | View Code Duplication | public function iShouldSeeASingleOrderFromCustomer(CustomerInterface $customer) |
|
| 128 | |||
| 129 | /** |
||
| 130 | * @Then it should have been placed by the customer :customerEmail |
||
| 131 | */ |
||
| 132 | public function itShouldBePlacedByCustomer($customerEmail) |
||
| 139 | |||
| 140 | /** |
||
| 141 | * @Then it should be shipped to :customerName, :street, :postcode, :city, :countryName |
||
| 142 | */ |
||
| 143 | public function itShouldBeShippedTo($customerName, $street, $postcode, $city, $countryName) |
||
| 150 | |||
| 151 | /** |
||
| 152 | * @Then it should be billed to :customerName, :street, :postcode, :city, :countryName |
||
| 153 | */ |
||
| 154 | View Code Duplication | public function itShouldBeBilledTo($customerName, $street, $postcode, $city, $countryName) |
|
| 161 | |||
| 162 | /** |
||
| 163 | * @Then it should be shipped via the :shippingMethodName shipping method |
||
| 164 | */ |
||
| 165 | public function itShouldBeShippedViaShippingMethod($shippingMethodName) |
||
| 172 | |||
| 173 | /** |
||
| 174 | * @Then it should be paid with :paymentMethodName |
||
| 175 | */ |
||
| 176 | public function itShouldBePaidWith($paymentMethodName) |
||
| 183 | |||
| 184 | /** |
||
| 185 | * @Then /^it should have (\d+) items$/ |
||
| 186 | */ |
||
| 187 | public function itShouldHaveAmountOfItems($amount) |
||
| 197 | |||
| 198 | /** |
||
| 199 | * @Then the product named :productName should be in the items list |
||
| 200 | */ |
||
| 201 | public function theProductShouldBeInTheItemsList($productName) |
||
| 208 | |||
| 209 | /** |
||
| 210 | * @Then the order's items total should be :itemsTotal |
||
| 211 | */ |
||
| 212 | public function theOrdersItemsTotalShouldBe($itemsTotal) |
||
| 222 | |||
| 223 | /** |
||
| 224 | * @Then the order's total should be :total |
||
| 225 | */ |
||
| 226 | public function theOrdersTotalShouldBe($total) |
||
| 236 | |||
| 237 | /** |
||
| 238 | * @Then there should be a shipping charge :shippingCharge |
||
| 239 | */ |
||
| 240 | public function theOrdersShippingChargesShouldBe($shippingCharge) |
||
| 247 | |||
| 248 | /** |
||
| 249 | * @Then the order's shipping total should be :shippingTotal |
||
| 250 | */ |
||
| 251 | public function theOrdersShippingTotalShouldBe($shippingTotal) |
||
| 261 | |||
| 262 | /** |
||
| 263 | * @Then the order should have tax :tax |
||
| 264 | */ |
||
| 265 | public function theOrderShouldHaveTax($tax) |
||
| 272 | |||
| 273 | /** |
||
| 274 | * @Then the order's tax total should be :taxTotal |
||
| 275 | */ |
||
| 276 | public function theOrdersTaxTotalShouldBe($taxTotal) |
||
| 286 | |||
| 287 | /** |
||
| 288 | * @Then the order's promotion discount should be :promotionDiscount |
||
| 289 | */ |
||
| 290 | public function theOrdersPromotionDiscountShouldBe($promotionDiscount) |
||
| 297 | |||
| 298 | /** |
||
| 299 | * @Then the order's promotion total should be :promotionTotal |
||
| 300 | */ |
||
| 301 | public function theOrdersPromotionTotalShouldBe($promotionTotal) |
||
| 311 | |||
| 312 | /** |
||
| 313 | * @When I check :itemName data |
||
| 314 | */ |
||
| 315 | public function iCheckData($itemName) |
||
| 319 | |||
| 320 | /** |
||
| 321 | * @Then /^(its) unit price should be ([^"]+)$/ |
||
| 322 | */ |
||
| 323 | public function itemUnitPriceShouldBe($itemName, $unitPrice) |
||
| 333 | |||
| 334 | /** |
||
| 335 | * @Then /^(its) discounted unit price should be ([^"]+)$/ |
||
| 336 | */ |
||
| 337 | public function itemDiscountedUnitPriceShouldBe($itemName, $discountedUnitPrice) |
||
| 347 | |||
| 348 | /** |
||
| 349 | * @Then /^(its) quantity should be ([^"]+)$/ |
||
| 350 | */ |
||
| 351 | public function itemQuantityShouldBe($itemName, $quantity) |
||
| 361 | |||
| 362 | /** |
||
| 363 | * @Then /^(its) subtotal should be ([^"]+)$/ |
||
| 364 | */ |
||
| 365 | public function itemSubtotalShouldBe($itemName, $subtotal) |
||
| 375 | |||
| 376 | /** |
||
| 377 | * @Then /^(its) discount should be ([^"]+)$/ |
||
| 378 | * @Then the :itemName should have :discount discount |
||
| 379 | */ |
||
| 380 | public function theItemShouldHaveDiscount($itemName, $discount) |
||
| 390 | |||
| 391 | /** |
||
| 392 | * @Then /^(its) tax should be ([^"]+)$/ |
||
| 393 | */ |
||
| 394 | public function itemTaxShouldBe($itemName, $tax) |
||
| 404 | |||
| 405 | /** |
||
| 406 | * @Then /^(its) total should be ([^"]+)$/ |
||
| 407 | */ |
||
| 408 | public function itemTotalShouldBe($itemName, $total) |
||
| 418 | |||
| 419 | /** |
||
| 420 | * @When I delete the order :order |
||
| 421 | */ |
||
| 422 | public function iDeleteOrder(OrderInterface $order) |
||
| 423 | { |
||
| 424 | $this->sharedStorage->set('order', $order); |
||
| 425 | |||
| 426 | $this->showPage->open(['id' => $order->getId()]); |
||
| 427 | $this->showPage->deleteOrder(); |
||
| 428 | } |
||
| 429 | |||
| 430 | /** |
||
| 431 | * @Then /^(this order) should not exist in the registry$/ |
||
| 432 | */ |
||
| 433 | public function orderShouldNotExistInTheRegistry(OrderInterface $order) |
||
| 442 | |||
| 443 | /** |
||
| 444 | * @Then I should be notified that the order's payment has been successfully completed |
||
| 445 | */ |
||
| 446 | public function iShouldBeNotifiedThatTheOrderSPaymentHasBeenSuccessfullyCompleted() |
||
| 450 | |||
| 451 | /** |
||
| 452 | * @Then it should have completed payment state |
||
| 453 | */ |
||
| 454 | public function itShouldHaveCompletedPaymentState() |
||
| 461 | |||
| 462 | /** |
||
| 463 | * @Then /^I should not be able to mark (this order) as paid again$/ |
||
| 464 | */ |
||
| 465 | public function iShouldNotBeAbleToFinalizeItsPayment(OrderInterface $order) |
||
| 472 | |||
| 473 | /** |
||
| 474 | * @Then I should be notified that the order's shipment has been successfully shipped |
||
| 475 | */ |
||
| 476 | public function iShouldBeNotifiedThatTheOrderSShipmentHasBeenSuccessfullyShipped() |
||
| 480 | |||
| 481 | /** |
||
| 482 | * @Then its shipment state should be :shipmentState |
||
| 483 | */ |
||
| 484 | public function itsShipmentStateShouldBe($shipmentState) |
||
| 491 | |||
| 492 | /** |
||
| 493 | * @Then /^I should not be able to ship (this order)$/ |
||
| 494 | */ |
||
| 495 | public function iShouldNotBeAbleToShipThisOrder(OrderInterface $order) |
||
| 502 | |||
| 503 | /** |
||
| 504 | * @When I cancel this order |
||
| 505 | */ |
||
| 506 | public function iCancelThisOrder() |
||
| 510 | |||
| 511 | /** |
||
| 512 | * @Then I should be notified that it has been successfully updated |
||
| 513 | */ |
||
| 514 | public function iShouldBeNotifiedAboutItHasBeenSuccessfullyCanceled() |
||
| 521 | |||
| 522 | /** |
||
| 523 | * @Then I should not be able to cancel this order |
||
| 524 | */ |
||
| 525 | public function iShouldNotBeAbleToCancelThisOrder() |
||
| 532 | |||
| 533 | /** |
||
| 534 | * @Then its state should be :state |
||
| 535 | */ |
||
| 536 | public function itsStateShouldBe($state) |
||
| 544 | |||
| 545 | /** |
||
| 546 | * @Then it should have a :state state |
||
| 547 | */ |
||
| 548 | public function itShouldHaveState($state) |
||
| 552 | |||
| 553 | /** |
||
| 554 | * @Then /^(the customer service) should know about (this additional note) for (this order made by "[^"]+")$/ |
||
| 555 | */ |
||
| 556 | public function theCustomerServiceShouldKnowAboutThisAdditionalNotes(UserInterface $user, $note, OrderInterface $order) |
||
| 563 | } |
||
| 564 |
Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation.
You can also find more detailed suggestions in the “Code” section of your repository.