Complex classes like ProductContext 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 ProductContext, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
28 | final class ProductContext implements Context |
||
29 | { |
||
30 | /** |
||
31 | * @var ShowPageInterface |
||
32 | */ |
||
33 | private $showPage; |
||
34 | |||
35 | /** |
||
36 | * @var IndexPageInterface |
||
37 | */ |
||
38 | private $indexPage; |
||
39 | |||
40 | /** |
||
41 | * @var ProductReviewIndexPageInterface |
||
42 | */ |
||
43 | private $productReviewsIndexPage; |
||
44 | |||
45 | /** |
||
46 | * @param ShowPageInterface $showPage |
||
47 | * @param IndexPageInterface $indexPage |
||
48 | * @param ProductReviewIndexPageInterface $productReviewsIndexPage |
||
49 | */ |
||
50 | public function __construct( |
||
59 | |||
60 | /** |
||
61 | * @Then I should be able to access product :product |
||
62 | */ |
||
63 | public function iShouldBeAbleToAccessProduct(ProductInterface $product) |
||
69 | |||
70 | /** |
||
71 | * @Then I should not be able to access product :product |
||
72 | */ |
||
73 | public function iShouldNotBeAbleToAccessProduct(ProductInterface $product) |
||
79 | |||
80 | /** |
||
81 | * @When /^I check (this product)'s details$/ |
||
82 | * @When /^I check (this product)'s details in the ("([^"]+)" locale)$/ |
||
83 | * @When I view product :product |
||
84 | * @When I view product :product in the :localeCode locale |
||
85 | */ |
||
86 | public function iOpenProductPage(ProductInterface $product, $localeCode = 'en_US') |
||
90 | |||
91 | /** |
||
92 | * @When /^I try to check (this product)'s details in the ("([^"]+)" locale)$/ |
||
93 | */ |
||
94 | public function iTryToOpenProductPage(ProductInterface $product, $localeCode = 'en_US') |
||
101 | |||
102 | /** |
||
103 | * @Then /^I should not be able to view (this product) in the ("([^"]+)" locale)$/ |
||
104 | */ |
||
105 | public function iShouldNotBeAbleToViewThisProductInLocale(ProductInterface $product, $localeCode = 'en_US') |
||
114 | |||
115 | /** |
||
116 | * @Then I should see the product name :name |
||
117 | */ |
||
118 | public function iShouldSeeProductName($name) |
||
122 | |||
123 | /** |
||
124 | * @When I open page :url |
||
125 | */ |
||
126 | public function iOpenPage($url) |
||
130 | |||
131 | /** |
||
132 | * @Then I should be on :product product detailed page |
||
133 | * @Then I should still be on product :product page |
||
134 | */ |
||
135 | public function iShouldBeOnProductDetailedPage(ProductInterface $product) |
||
139 | |||
140 | /** |
||
141 | * @Then I should (also) see the product attribute :attributeName with value :expectedAttribute |
||
142 | */ |
||
143 | public function iShouldSeeTheProductAttributeWithValue($attributeName, $expectedAttribute) |
||
147 | |||
148 | /** |
||
149 | * @Then I should (also) see the product attribute :attributeName with date :expectedAttribute |
||
150 | */ |
||
151 | public function iShouldSeeTheProductAttributeWithDate($attributeName, $expectedAttribute) |
||
158 | |||
159 | /** |
||
160 | * @Then I should see :count attributes |
||
161 | */ |
||
162 | public function iShouldSeeAttributes($count) |
||
166 | |||
167 | /** |
||
168 | * @Then the first attribute should be :name |
||
169 | */ |
||
170 | public function theFirstAttributeShouldBe($name) |
||
176 | |||
177 | /** |
||
178 | * @Then the last attribute should be :name |
||
179 | */ |
||
180 | public function theLastAttributeShouldBe($name) |
||
186 | |||
187 | /** |
||
188 | * @When /^I browse products from (taxon "([^"]+)")$/ |
||
189 | */ |
||
190 | public function iCheckListOfProductsForTaxon(TaxonInterface $taxon) |
||
194 | |||
195 | /** |
||
196 | * @When I search for products with name :name |
||
197 | */ |
||
198 | public function iSearchForProductsWithName($name) |
||
202 | |||
203 | /** |
||
204 | * @When I sort products by the lowest price first |
||
205 | */ |
||
206 | public function iSortProductsByTheLowestPriceFirst() |
||
210 | |||
211 | /** |
||
212 | * @When I sort products by the highest price first |
||
213 | */ |
||
214 | public function iSortProductsByTheHighestPriceFisrt() |
||
218 | |||
219 | /** |
||
220 | * @When I sort products alphabetically from a to z |
||
221 | */ |
||
222 | public function iSortProductsAlphabeticallyFromAToZ() |
||
226 | |||
227 | /** |
||
228 | * @When I sort products alphabetically from z to a |
||
229 | */ |
||
230 | public function iSortProductsAlphabeticallyFromZToA() |
||
234 | |||
235 | /** |
||
236 | * @When I clear filter |
||
237 | */ |
||
238 | public function iClearFilter() |
||
242 | |||
243 | /** |
||
244 | * @Then I should see the product :productName |
||
245 | */ |
||
246 | public function iShouldSeeProduct($productName) |
||
250 | |||
251 | /** |
||
252 | * @Then I should not see the product :productName |
||
253 | */ |
||
254 | public function iShouldNotSeeProduct($productName) |
||
258 | |||
259 | /** |
||
260 | * @Then I should see empty list of products |
||
261 | */ |
||
262 | public function iShouldSeeEmptyListOfProducts() |
||
266 | |||
267 | /** |
||
268 | * @Then I should see that it is out of stock |
||
269 | */ |
||
270 | public function iShouldSeeItIsOutOfStock() |
||
274 | |||
275 | /** |
||
276 | * @Then I should be unable to add it to the cart |
||
277 | */ |
||
278 | public function iShouldBeUnableToAddItToTheCart() |
||
282 | |||
283 | /** |
||
284 | * @Then the product price should be :price |
||
285 | * @Then I should see the product price :price |
||
286 | */ |
||
287 | public function iShouldSeeTheProductPrice($price) |
||
291 | |||
292 | /** |
||
293 | * @When I set its :optionName to :optionValue |
||
294 | */ |
||
295 | public function iSetItsOptionTo($optionName, $optionValue) |
||
299 | |||
300 | /** |
||
301 | * @When I select :variantName variant |
||
302 | */ |
||
303 | public function iSelectVariant($variantName) |
||
307 | |||
308 | /** |
||
309 | * @Then its current variant should be named :name |
||
310 | */ |
||
311 | public function itsCurrentVariantShouldBeNamed($name) |
||
315 | |||
316 | /** |
||
317 | * @Then I should see the product :productName with price :productPrice |
||
318 | */ |
||
319 | public function iShouldSeeTheProductWithPrice($productName, $productPrice) |
||
323 | |||
324 | /** |
||
325 | * @Then /^I should be notified that (this product) does not have sufficient stock$/ |
||
326 | */ |
||
327 | public function iShouldBeNotifiedThatThisProductDoesNotHaveSufficientStock(ProductInterface $product) |
||
331 | |||
332 | /** |
||
333 | * @Then /^I should not be notified that (this product) does not have sufficient stock$/ |
||
334 | */ |
||
335 | public function iShouldNotBeNotifiedThatThisProductDoesNotHaveSufficientStock(ProductInterface $product) |
||
339 | |||
340 | /** |
||
341 | * @Then I should see a main image |
||
342 | */ |
||
343 | public function iShouldSeeAMainImage() |
||
347 | |||
348 | /** |
||
349 | * @When /^I view (oldest|newest) products from (taxon "([^"]+)")$/ |
||
350 | */ |
||
351 | public function iViewSortedProductsFromTaxon($sortDirection, TaxonInterface $taxon) |
||
357 | |||
358 | /** |
||
359 | * @Then I should see :numberOfProducts products in the list |
||
360 | */ |
||
361 | public function iShouldSeeProductsInTheList($numberOfProducts) |
||
365 | |||
366 | /** |
||
367 | * @Then I should see a product with name :name |
||
368 | */ |
||
369 | public function iShouldSeeProductWithName($name) |
||
373 | |||
374 | /** |
||
375 | * @Then the first product on the list should have name :name |
||
376 | */ |
||
377 | public function theFirstProductOnTheListShouldHaveName($name) |
||
381 | |||
382 | /** |
||
383 | * @Then the first product on the list should have name :name and price :price |
||
384 | */ |
||
385 | public function theFirstProductOnTheListShouldHaveNameAndPrice($name, $price) |
||
390 | |||
391 | /** |
||
392 | * @Then the last product on the list should have name :name |
||
393 | */ |
||
394 | public function theLastProductOnTheListShouldHaveName($name) |
||
398 | |||
399 | /** |
||
400 | * @Then the last product on the list should have name :name and price :price |
||
401 | */ |
||
402 | public function theLastProductOnTheListShouldHaveNameAndPrice($name, $price) |
||
407 | |||
408 | /** |
||
409 | * @Then I should see :count product reviews |
||
410 | */ |
||
411 | public function iShouldSeeProductReviews($count) |
||
415 | |||
416 | /** |
||
417 | * @Then I should see reviews titled :firstReview, :secondReview and :thirdReview |
||
418 | */ |
||
419 | public function iShouldSeeReviewsTitled(...$reviews) |
||
428 | |||
429 | /** |
||
430 | * @Then I should not see review titled :title |
||
431 | */ |
||
432 | public function iShouldNotSeeReviewTitled($title) |
||
436 | |||
437 | /** |
||
438 | * @When /^I check (this product)'s reviews$/ |
||
439 | */ |
||
440 | public function iCheckThisProductSReviews(ProductInterface $product) |
||
444 | |||
445 | /** |
||
446 | * @Then /^I should see (\d+) product reviews in the list$/ |
||
447 | */ |
||
448 | public function iShouldSeeNumberOfProductReviewsInTheList($count) |
||
452 | |||
453 | /** |
||
454 | * @Then I should not see review titled :title in the list |
||
455 | */ |
||
456 | public function iShouldNotSeeReviewTitledInTheList($title) |
||
460 | |||
461 | /** |
||
462 | * @Then /^I should be notified that there are no reviews$/ |
||
463 | */ |
||
464 | public function iShouldBeNotifiedThatThereAreNoReviews() |
||
468 | |||
469 | /** |
||
470 | * @Then I should see :rating as its average rating |
||
471 | */ |
||
472 | public function iShouldSeeAsItsAverageRating($rating) |
||
476 | |||
477 | /** |
||
478 | * @Then /^I should(?:| also) see the product association "([^"]+)" with (products "[^"]+" and "[^"]+")$/ |
||
479 | */ |
||
480 | public function iShouldSeeTheProductAssociationWithProducts($productAssociationName, array $products) |
||
491 | |||
492 | /** |
||
493 | * @Then /^average rating of (product "[^"]+") should be (\d+)$/ |
||
494 | */ |
||
495 | public function thisProductAverageRatingShouldBe(ProductInterface $product, $averageRating) |
||
500 | |||
501 | /** |
||
502 | * @Then they should have order like :firstProductName, :secondProductName and :thirdProductName |
||
503 | */ |
||
504 | public function theyShouldHaveOrderLikeAnd(...$productNames) |
||
508 | |||
509 | /** |
||
510 | * @param string $productName |
||
511 | * @param string $productAssociationName |
||
512 | * |
||
513 | * @throws \InvalidArgumentException |
||
514 | */ |
||
515 | private function assertProductIsInAssociation($productName, $productAssociationName) |
||
526 | |||
527 | /** |
||
528 | * @return NodeElement[] |
||
529 | * |
||
530 | * @throws \InvalidArgumentException |
||
531 | */ |
||
532 | private function getProductAttributes() |
||
539 | } |
||
540 |
Let’s take a look at an example:
In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.
Available Fixes
Change the type-hint for the parameter:
Add an additional type-check:
Add the method to the interface: