Complex classes like ManagingProductVariantsContext 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 ManagingProductVariantsContext, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
29 | final class ManagingProductVariantsContext implements Context |
||
30 | { |
||
31 | /** |
||
32 | * @var SharedStorageInterface |
||
33 | */ |
||
34 | private $sharedStorage; |
||
35 | |||
36 | /** |
||
37 | * @var CreatePageInterface |
||
38 | */ |
||
39 | private $createPage; |
||
40 | |||
41 | /** |
||
42 | * @var IndexPageInterface |
||
43 | */ |
||
44 | private $indexPage; |
||
45 | |||
46 | /** |
||
47 | * @var UpdatePageInterface |
||
48 | */ |
||
49 | private $updatePage; |
||
50 | |||
51 | /** |
||
52 | * @var GeneratePageInterface |
||
53 | */ |
||
54 | private $generatePage; |
||
55 | |||
56 | /** |
||
57 | * @var CurrentPageResolverInterface |
||
58 | */ |
||
59 | private $currentPageResolver; |
||
60 | |||
61 | /** |
||
62 | * @var NotificationCheckerInterface |
||
63 | */ |
||
64 | private $notificationChecker; |
||
65 | |||
66 | /** |
||
67 | * @param SharedStorageInterface $sharedStorage |
||
68 | * @param CreatePageInterface $createPage |
||
69 | * @param IndexPageInterface $indexPage |
||
70 | * @param UpdatePageInterface $updatePage |
||
71 | * @param GeneratePageInterface $generatePage |
||
72 | * @param CurrentPageResolverInterface $currentPageResolver |
||
73 | * @param NotificationCheckerInterface $notificationChecker |
||
74 | */ |
||
75 | public function __construct( |
||
92 | |||
93 | /** |
||
94 | * @Given /^I want to create a new variant of (this product)$/ |
||
95 | */ |
||
96 | public function iWantToCreateANewProduct(ProductInterface $product) |
||
100 | |||
101 | /** |
||
102 | * @When I specify its code as :code |
||
103 | * @When I do not specify its code |
||
104 | */ |
||
105 | public function iSpecifyItsCodeAs($code = null) |
||
109 | |||
110 | /** |
||
111 | * @When I name it :name in :language |
||
112 | */ |
||
113 | public function iNameItIn($name, $language) |
||
117 | |||
118 | /** |
||
119 | * @When I add it |
||
120 | * @When I try to add it |
||
121 | */ |
||
122 | public function iAddIt() |
||
123 | { |
||
124 | $this->createPage->create(); |
||
125 | } |
||
126 | |||
127 | /** |
||
128 | * @When I disable its inventory tracking |
||
129 | */ |
||
130 | public function iDisableItsTracking() |
||
131 | { |
||
132 | $this->updatePage->disableTracking(); |
||
133 | } |
||
134 | |||
135 | /** |
||
136 | * @When I enable its inventory tracking |
||
137 | */ |
||
138 | public function iEnableItsTracking() |
||
139 | { |
||
140 | $this->updatePage->enableTracking(); |
||
141 | } |
||
142 | |||
143 | /** |
||
144 | * @When /^I set its(?:| default) price to "(?:€|£|\$)([^"]+)" for "([^"]+)" channel$/ |
||
145 | * @When I do not set its price |
||
146 | */ |
||
147 | public function iSetItsPriceTo($price = null, $channelName = null) |
||
148 | { |
||
149 | $this->createPage->specifyPrice($price, $channelName ?? $this->sharedStorage->get('channel')); |
||
150 | } |
||
151 | |||
152 | /** |
||
153 | * @When /^I set its original price to "(?:€|£|\$)([^"]+)" for "([^"]+)" channel$/ |
||
154 | */ |
||
155 | public function iSetItsOriginalPriceTo($originalPrice, $channelName) |
||
156 | { |
||
157 | $this->createPage->specifyOriginalPrice($originalPrice, $channelName); |
||
158 | } |
||
159 | |||
160 | /** |
||
161 | * @When I set its height, width, depth and weight to :number |
||
162 | */ |
||
163 | public function iSetItsDimensionsTo($value) |
||
164 | { |
||
165 | $this->createPage->specifyHeightWidthDepthAndWeight($value, $value, $value, $value); |
||
166 | } |
||
167 | |||
168 | /** |
||
169 | * @When I do not specify its current stock |
||
170 | */ |
||
171 | public function iDoNetSetItsCurrentStockTo() |
||
172 | { |
||
173 | $this->createPage->specifyCurrentStock(''); |
||
174 | } |
||
175 | |||
176 | /** |
||
177 | * @When I choose :calculatorName calculator |
||
178 | */ |
||
179 | public function iChooseCalculator($calculatorName) |
||
180 | { |
||
181 | $this->createPage->choosePricingCalculator($calculatorName); |
||
182 | } |
||
183 | |||
184 | /** |
||
185 | * @When I set its :optionName option to :optionValue |
||
186 | */ |
||
187 | public function iSetItsOptionAs($optionName, $optionValue) |
||
188 | { |
||
189 | $this->createPage->selectOption($optionName, $optionValue); |
||
190 | } |
||
191 | |||
192 | /** |
||
193 | * @When I set the position of :name to :position |
||
194 | */ |
||
195 | public function iSetThePositionOfTo($name, $position) |
||
196 | { |
||
197 | $this->indexPage->setPosition($name, $position); |
||
198 | } |
||
199 | |||
200 | /** |
||
201 | * @When I save my new configuration |
||
202 | */ |
||
203 | public function iSaveMyNewConfiguration() |
||
204 | { |
||
205 | $this->indexPage->savePositions(); |
||
206 | } |
||
207 | |||
208 | /** |
||
209 | * @When I do not want to have shipping required for this product |
||
210 | */ |
||
211 | public function iDoNotWantToHaveShippingRequiredForThisProduct() |
||
212 | { |
||
213 | $this->createPage->setShippingRequired(false); |
||
214 | } |
||
215 | |||
216 | /** |
||
217 | * @When I check (also) the :productVariantName product variant |
||
218 | */ |
||
219 | public function iCheckTheProductVariantName(string $productVariantName): void |
||
220 | { |
||
221 | $this->indexPage->checkResourceOnPage(['name' => $productVariantName]); |
||
222 | } |
||
223 | |||
224 | /** |
||
225 | * @When I delete them |
||
226 | */ |
||
227 | public function iDeleteThem(): void |
||
228 | { |
||
229 | $this->indexPage->bulkDelete(); |
||
230 | } |
||
231 | |||
232 | /** |
||
233 | * @Then /^the (variant with code "[^"]+") should be priced at (?:€|£|\$)([^"]+) for channel "([^"]+)"$/ |
||
234 | */ |
||
235 | public function theVariantWithCodeShouldBePricedAtForChannel(ProductVariantInterface $productVariant, $price, $channelName) |
||
236 | { |
||
237 | $this->updatePage->open(['id' => $productVariant->getId(), 'productId' => $productVariant->getProduct()->getId()]); |
||
238 | |||
239 | Assert::same($this->updatePage->getPriceForChannel($channelName), $price); |
||
240 | } |
||
241 | |||
242 | /** |
||
243 | * @Then /^the (variant with code "[^"]+") should be named "([^"]+)" in ("([^"]+)" locale)$/ |
||
244 | */ |
||
245 | public function theVariantWithCodeShouldBeNamedIn(ProductVariantInterface $productVariant, $name, $language) |
||
246 | { |
||
247 | $this->updatePage->open(['id' => $productVariant->getId(), 'productId' => $productVariant->getProduct()->getId()]); |
||
248 | |||
249 | Assert::same($this->updatePage->getNameInLanguage($language), $name); |
||
250 | } |
||
251 | |||
252 | /** |
||
253 | * @Then /^the (variant with code "[^"]+") should have an original price of (?:€|£|\$)([^"]+) for channel "([^"]+)"$/ |
||
254 | */ |
||
255 | public function theVariantWithCodeShouldHaveAnOriginalPriceOfForChannel(ProductVariantInterface $productVariant, $originalPrice, $channelName) |
||
256 | { |
||
257 | $this->updatePage->open(['id' => $productVariant->getId(), 'productId' => $productVariant->getProduct()->getId()]); |
||
258 | |||
259 | Assert::same( |
||
260 | $this->updatePage->getOriginalPriceForChannel($channelName), |
||
261 | $originalPrice |
||
262 | ); |
||
263 | } |
||
264 | |||
265 | /** |
||
266 | * @When /^I delete the ("[^"]+" variant of product "[^"]+")$/ |
||
267 | * @When /^I try to delete the ("[^"]+" variant of product "[^"]+")$/ |
||
268 | */ |
||
269 | public function iDeleteTheVariantOfProduct(ProductVariantInterface $productVariant) |
||
270 | { |
||
271 | $this->indexPage->open(['productId' => $productVariant->getProduct()->getId()]); |
||
272 | |||
273 | $this->indexPage->deleteResourceOnPage(['code' => $productVariant->getCode()]); |
||
274 | } |
||
275 | |||
276 | /** |
||
277 | * @Then I should be notified that this variant is in use and cannot be deleted |
||
278 | */ |
||
279 | public function iShouldBeNotifiedOfFailure() |
||
280 | { |
||
281 | $this->notificationChecker->checkNotification( |
||
282 | 'Cannot delete, the product variant is in use.', |
||
283 | NotificationType::failure() |
||
284 | ); |
||
285 | } |
||
286 | |||
287 | /** |
||
288 | * @When /^I want to modify the ("[^"]+" product variant)$/ |
||
289 | */ |
||
290 | public function iWantToModifyAProduct(ProductVariantInterface $productVariant) |
||
291 | { |
||
292 | $this->updatePage->open(['id' => $productVariant->getId(), 'productId' => $productVariant->getProduct()->getId()]); |
||
293 | } |
||
294 | |||
295 | /** |
||
296 | * @Then the code field should be disabled |
||
297 | */ |
||
298 | public function theCodeFieldShouldBeDisabled() |
||
299 | { |
||
300 | Assert::true($this->updatePage->isCodeDisabled()); |
||
301 | } |
||
302 | |||
303 | /** |
||
304 | * @Then I should be notified that :element is required |
||
305 | */ |
||
306 | public function iShouldBeNotifiedThatIsRequired($element) |
||
307 | { |
||
308 | $this->assertValidationMessage($element, sprintf('Please enter the %s.', $element)); |
||
309 | } |
||
310 | |||
311 | /** |
||
312 | * @Then I should be notified that code has to be unique |
||
313 | */ |
||
314 | public function iShouldBeNotifiedThatCodeHasToBeUnique() |
||
315 | { |
||
316 | $this->assertValidationMessage('code', 'Product variant code must be unique.'); |
||
317 | } |
||
318 | |||
319 | /** |
||
320 | * @Then I should be notified that current stock is required |
||
321 | */ |
||
322 | public function iShouldBeNotifiedThatOnHandIsRequired() |
||
323 | { |
||
324 | $this->assertValidationMessage('on_hand', 'Please enter on hand.'); |
||
325 | } |
||
326 | |||
327 | /** |
||
328 | * @Then I should be notified that height, width, depth and weight cannot be lower than 0 |
||
329 | */ |
||
330 | public function iShouldBeNotifiedThatIsHeightWidthDepthWeightCannotBeLowerThan() |
||
331 | { |
||
332 | $this->assertValidationMessage('height', 'Height cannot be negative.'); |
||
333 | $this->assertValidationMessage('width', 'Width cannot be negative.'); |
||
334 | $this->assertValidationMessage('depth', 'Depth cannot be negative.'); |
||
335 | $this->assertValidationMessage('weight', 'Weight cannot be negative.'); |
||
336 | } |
||
337 | |||
338 | /** |
||
339 | * @Then I should be notified that price cannot be lower than 0.01 |
||
340 | */ |
||
341 | public function iShouldBeNotifiedThatPriceCannotBeLowerThen() |
||
342 | { |
||
343 | /** @var CreatePageInterface|UpdatePageInterface $currentPage */ |
||
344 | $currentPage = $this->currentPageResolver->getCurrentPageWithForm([$this->createPage, $this->updatePage]); |
||
345 | |||
346 | Assert::contains($currentPage->getPricesValidationMessage(), 'Price must be at least 0.01.'); |
||
|
|||
347 | } |
||
348 | |||
349 | /** |
||
350 | * @Then I should be notified that this variant already exists |
||
351 | */ |
||
352 | public function iShouldBeNotifiedThatThisVariantAlreadyExists() |
||
353 | { |
||
354 | /** @var CreatePageInterface|UpdatePageInterface $currentPage */ |
||
355 | $currentPage = $this->currentPageResolver->getCurrentPageWithForm([$this->createPage, $this->updatePage]); |
||
356 | |||
357 | Assert::same($currentPage->getValidationMessageForForm(), 'Variant with this option set already exists.'); |
||
358 | } |
||
359 | |||
360 | /** |
||
361 | * @Then /^I should be notified that code is required for the (\d)(?:st|nd|rd|th) variant$/ |
||
362 | */ |
||
363 | public function iShouldBeNotifiedThatCodeIsRequiredForVariant($position) |
||
364 | { |
||
365 | Assert::same( |
||
366 | $this->generatePage->getValidationMessage('code', $position - 1), |
||
367 | 'Please enter the code.' |
||
368 | ); |
||
369 | } |
||
370 | |||
371 | /** |
||
372 | * @Then /^I should be notified that prices in all channels must be defined for the (\d)(?:st|nd|rd|th) variant$/ |
||
373 | */ |
||
374 | public function iShouldBeNotifiedThatPricesInAllChannelsMustBeDefinedForTheVariant($position) |
||
375 | { |
||
376 | Assert::same( |
||
377 | $this->generatePage->getPricesValidationMessage($position - 1), |
||
378 | 'You must define price for every channel.' |
||
379 | ); |
||
380 | } |
||
381 | |||
382 | /** |
||
383 | * @Then /^I should be notified that variant code must be unique within this product for the (\d)(?:st|nd|rd|th) variant$/ |
||
384 | */ |
||
385 | public function iShouldBeNotifiedThatVariantCodeMustBeUniqueWithinThisProductForYheVariant($position) |
||
386 | { |
||
387 | Assert::same( |
||
388 | $this->generatePage->getValidationMessage('code', $position - 1), |
||
389 | 'This code must be unique within this product.' |
||
390 | ); |
||
391 | } |
||
392 | |||
393 | /** |
||
394 | * @Then I should be notified that prices in all channels must be defined |
||
395 | */ |
||
396 | public function iShouldBeNotifiedThatPricesInAllChannelsMustBeDefined() |
||
397 | { |
||
398 | Assert::contains( |
||
399 | $this->createPage->getPricesValidationMessage(), |
||
400 | 'You must define price for every channel.' |
||
401 | ); |
||
402 | } |
||
403 | |||
404 | /** |
||
405 | * @When I save my changes |
||
406 | * @When I try to save my changes |
||
407 | */ |
||
408 | public function iSaveMyChanges() |
||
409 | { |
||
410 | $this->updatePage->saveChanges(); |
||
411 | } |
||
412 | |||
413 | /** |
||
414 | * @Then /^inventory of (this variant) should not be tracked$/ |
||
415 | */ |
||
416 | public function thisProductVariantShouldNotBeTracked(ProductVariantInterface $productVariant) |
||
422 | |||
423 | /** |
||
424 | * @Then /^inventory of (this variant) should be tracked$/ |
||
425 | */ |
||
426 | public function thisProductVariantShouldBeTracked(ProductVariantInterface $productVariant) |
||
432 | |||
433 | /** |
||
434 | * @When /^I want to generate new variants for (this product)$/ |
||
435 | */ |
||
436 | public function iWantToGenerateNewVariantsForThisProduct(ProductInterface $product) |
||
440 | |||
441 | /** |
||
442 | * @When I generate it |
||
443 | * @When I try to generate it |
||
444 | */ |
||
445 | public function iClickGenerate() |
||
449 | |||
450 | /** |
||
451 | * @When /^I specify that the (\d)(?:st|nd|rd|th) variant is identified by "([^"]+)" code and costs "(?:€|£|\$)([^"]+)" in ("[^"]+") channel$/ |
||
452 | */ |
||
453 | public function iSpecifyThereAreVariantsIdentifiedByCodeWithCost($nthVariant, $code, $price, $channelName) |
||
458 | |||
459 | /** |
||
460 | * @When /^I specify that the (\d)(?:st|nd|rd|th) variant is identified by "([^"]+)" code$/ |
||
461 | */ |
||
462 | public function iSpecifyThereAreVariantsIdentifiedByCode($nthVariant, $code) |
||
466 | |||
467 | /** |
||
468 | * @When /^I specify that the (\d)(?:st|nd|rd|th) variant costs "(?:€|£|\$)([^"]+)" in ("[^"]+") channel$/ |
||
469 | */ |
||
470 | public function iSpecifyThereAreVariantsWithCost($nthVariant, $price, $channelName) |
||
474 | |||
475 | /** |
||
476 | * @When /^I remove (\d)(?:st|nd|rd|th) variant from the list$/ |
||
477 | */ |
||
478 | public function iRemoveVariantFromTheList($nthVariant) |
||
482 | |||
483 | /** |
||
484 | * @Then I should be notified that it has been successfully generated |
||
485 | */ |
||
486 | public function iShouldBeNotifiedThatItHasBeenSuccessfullyGenerated() |
||
490 | |||
491 | /** |
||
492 | * @When I set its shipping category as :shippingCategoryName |
||
493 | */ |
||
494 | public function iSetItsShippingCategoryAs($shippingCategoryName) |
||
498 | |||
499 | /** |
||
500 | * @When I do not specify any information about variants |
||
501 | */ |
||
502 | public function iDoNotSpecifyAnyInformationAboutVariants() |
||
506 | |||
507 | /** |
||
508 | * @When I change its quantity of inventory to :amount |
||
509 | */ |
||
510 | public function iChangeItsQuantityOfInventoryTo($amount) |
||
514 | |||
515 | /** |
||
516 | * @Then /^the (variant with code "[^"]+") should not have shipping required$/ |
||
517 | */ |
||
518 | public function theVariantWithCodeShouldNotHaveShippingRequired(ProductVariantInterface $productVariant) |
||
524 | |||
525 | /** |
||
526 | * @Then I should be notified that on hand quantity must be greater than the number of on hold units |
||
527 | */ |
||
528 | public function iShouldBeNotifiedThatOnHandQuantityMustBeGreaterThanTheNumberOfOnHoldUnits() |
||
535 | |||
536 | /** |
||
537 | * @param string $element |
||
538 | * @param string $message |
||
539 | */ |
||
540 | private function assertValidationMessage($element, $message) |
||
547 | } |
||
548 |
It seems like the method you are trying to call exists only in some of the possible types.
Let’s take a look at an example:
Available Fixes
Add an additional type-check:
Only allow a single type to be passed if the variable comes from a parameter: