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 ImageTest 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 ImageTest, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
12 | final class ImageTest extends TestCase |
||
13 | { |
||
14 | private $sourceFilesDir; |
||
15 | private $tempDir; |
||
16 | |||
17 | public function setUp() |
||
18 | { |
||
19 | $this->sourceFilesDir = __DIR__ . '/_files'; |
||
20 | $this->tempDir = sys_get_temp_dir() . '/imageUtilTest'; |
||
21 | if (is_dir($this->tempDir)) { |
||
22 | foreach (glob("{$this->tempDir}/*") as $file) { |
||
23 | unlink($file); |
||
24 | } |
||
25 | |||
26 | rmdir($this->tempDir); |
||
27 | } |
||
28 | } |
||
29 | |||
30 | /** |
||
31 | * Downsize ratio 2.0 to 0.25 |
||
32 | * |
||
33 | * @test |
||
34 | * @covers ::resize |
||
35 | */ |
||
36 | public function resizeDownsizeToMoreVerticalAspect() |
||
37 | { |
||
38 | $source = new \Imagick('pattern:gray0'); |
||
39 | $source->scaleImage(100, 50); |
||
40 | |||
41 | $imagick = Image::resize($source, 10, 40, ['color' => 'white', 'maxWidth' => 10000, 'maxHeight' => 10000]); |
||
42 | |||
43 | //making sure source didnt resize |
||
44 | $this->assertSame(100, $source->getImageWidth()); |
||
45 | $this->assertSame(50, $source->getImageHeight()); |
||
46 | |||
47 | $this->assertSame(10, $imagick->getImageWidth()); |
||
48 | $this->assertSame(40, $imagick->getImageHeight()); |
||
49 | |||
50 | $whiteBarTop = $imagick->getImagePixelColor(4, 16)->getHsl(); |
||
51 | $whiteBarBottom = $imagick->getImagePixelColor(4, 22)->getHsl(); |
||
52 | |||
53 | $imageLeft = $imagick->getImagePixelColor(0, 19)->getHsl(); |
||
54 | $imageRight = $imagick->getImagePixelColor(9, 19)->getHsl(); |
||
55 | $imageTop = $imagick->getImagePixelColor(4, 17)->getHsl(); |
||
56 | $imageBottom = $imagick->getImagePixelColor(4, 21)->getHsl(); |
||
57 | |||
58 | $this->assertGreaterThan(0.9, $whiteBarTop['luminosity']); |
||
59 | $this->assertGreaterThan(0.9, $whiteBarBottom['luminosity']); |
||
60 | |||
61 | $this->assertLessThan(0.1, $imageLeft['luminosity']); |
||
62 | $this->assertLessThan(0.1, $imageRight['luminosity']); |
||
63 | $this->assertLessThan(0.1, $imageTop['luminosity']); |
||
64 | $this->assertLessThan(0.1, $imageBottom['luminosity']); |
||
65 | } |
||
66 | |||
67 | /** |
||
68 | * Downsize ratio 2.0 to 4.0 |
||
69 | * |
||
70 | * @test |
||
71 | * @covers ::resize |
||
72 | */ |
||
73 | View Code Duplication | public function resizeDownsizeToMoreHorizontalAspect() |
|
103 | |||
104 | /** |
||
105 | * Upsize ratio 2.0 to 4.0 |
||
106 | * |
||
107 | * @test |
||
108 | * @covers ::resize |
||
109 | */ |
||
110 | View Code Duplication | public function resizeUpsizeToMoreHorizontalAspectWithoutGrow() |
|
140 | |||
141 | /** |
||
142 | * Upsize ratio 2.0 to 4.0 |
||
143 | * |
||
144 | * @test |
||
145 | * @covers ::resize |
||
146 | */ |
||
147 | View Code Duplication | public function resizeUpsizeToMoreHorizontalAspectWithGrow() |
|
177 | |||
178 | /** |
||
179 | * Upsize ratio 2.0 to 4.0 |
||
180 | * |
||
181 | * @test |
||
182 | * @covers ::resize |
||
183 | */ |
||
184 | View Code Duplication | public function resizeUpsizeToMoreVerticalAspect() |
|
214 | |||
215 | /** |
||
216 | * @test |
||
217 | * @covers ::resize |
||
218 | */ |
||
219 | public function resizeWithUpsizeAndBestFit() |
||
228 | |||
229 | /** |
||
230 | * @test |
||
231 | * @covers ::resize |
||
232 | */ |
||
233 | View Code Duplication | public function resizeWithColorOfBlur() |
|
240 | |||
241 | /** |
||
242 | * @test |
||
243 | * @covers ::resize |
||
244 | */ |
||
245 | View Code Duplication | public function resizeWithBlurBackground() |
|
252 | |||
253 | /** |
||
254 | * @test |
||
255 | * @covers ::resize |
||
256 | */ |
||
257 | View Code Duplication | public function resizeWithBurredBackgroundWithCustomBlurValue() |
|
265 | |||
266 | /** |
||
267 | * @test |
||
268 | * @covers ::resize |
||
269 | * @covers ::resizeMulti |
||
270 | * @expectedException \InvalidArgumentException |
||
271 | * @expectedExceptionMessage a $boxSizes width was not between 0 and $options["maxWidth"] |
||
272 | */ |
||
273 | public function resizeZeroBoxWidth() |
||
277 | |||
278 | /** |
||
279 | * @test |
||
280 | * @covers ::resize |
||
281 | * @covers ::resizeMulti |
||
282 | * @expectedException \InvalidArgumentException |
||
283 | * @expectedExceptionMessage a $boxSizes width was not between 0 and $options["maxWidth"] |
||
284 | */ |
||
285 | public function resizeLargeBoxWidth() |
||
289 | |||
290 | /** |
||
291 | * @test |
||
292 | * @covers ::resize |
||
293 | * @covers ::resizeMulti |
||
294 | * @expectedException \InvalidArgumentException |
||
295 | * @expectedExceptionMessage a $boxSizes height was not between 0 and $options["maxHeight"] |
||
296 | */ |
||
297 | public function resizeZeroBoxHeight() |
||
301 | |||
302 | /** |
||
303 | * @test |
||
304 | * @covers ::resize |
||
305 | * @covers ::resizeMulti |
||
306 | * @expectedException \InvalidArgumentException |
||
307 | * @expectedExceptionMessage a $boxSizes height was not between 0 and $options["maxHeight"] |
||
308 | */ |
||
309 | public function resizeLargeBoxHeight() |
||
313 | |||
314 | /** |
||
315 | * @test |
||
316 | * @covers ::resize |
||
317 | * @covers ::resizeMulti |
||
318 | * @expectedException \InvalidArgumentException |
||
319 | * @expectedExceptionMessage $options["color"] was not a string |
||
320 | */ |
||
321 | public function resizeNonStringColor() |
||
325 | |||
326 | /** |
||
327 | * @test |
||
328 | * @covers ::resize |
||
329 | * @covers ::resizeMulti |
||
330 | * @expectedException \InvalidArgumentException |
||
331 | * @expectedExceptionMessage $options["maxWidth"] was not an int |
||
332 | */ |
||
333 | public function resizeonIntMaxWidth() |
||
337 | |||
338 | /** |
||
339 | * @test |
||
340 | * @covers ::resize |
||
341 | * @covers ::resizeMulti |
||
342 | * @expectedException \InvalidArgumentException |
||
343 | * @expectedExceptionMessage $options["maxHeight"] was not an int |
||
344 | */ |
||
345 | public function resizeNonIntMaxHeight() |
||
349 | |||
350 | /** |
||
351 | * @test |
||
352 | * @covers ::resize |
||
353 | * @covers ::resizeMulti |
||
354 | * @expectedException \InvalidArgumentException |
||
355 | * @expectedExceptionMessage $options["upsize"] was not a bool |
||
356 | */ |
||
357 | public function resizeNonBoolUpsize() |
||
361 | |||
362 | /** |
||
363 | * @test |
||
364 | * @covers ::resize |
||
365 | * @covers ::resizeMulti |
||
366 | * @expectedException \InvalidArgumentException |
||
367 | * @expectedExceptionMessage $options["bestfit"] was not a bool |
||
368 | */ |
||
369 | public function resizeNonBoolBestFit() |
||
373 | |||
374 | /** |
||
375 | * Verify images are rotated according to EXIF header |
||
376 | * @test |
||
377 | * @covers ::resize |
||
378 | */ |
||
379 | public function resizeOrientation() |
||
414 | |||
415 | /** |
||
416 | * Downsize ratio 2.0 to 0.25 and 2.0 to 4.0 |
||
417 | * |
||
418 | * @test |
||
419 | * @covers ::resizeMulti |
||
420 | */ |
||
421 | public function resizeMultiDownsizeToMoreVerticalAndMoreHorizontalAspect() |
||
476 | |||
477 | /** |
||
478 | * @test |
||
479 | * @covers ::resizeMulti |
||
480 | */ |
||
481 | public function resizeMultiPerformance() |
||
509 | |||
510 | /** |
||
511 | * @test |
||
512 | * @covers ::resizeMulti |
||
513 | * @expectedException \InvalidArgumentException |
||
514 | * @expectedExceptionMessage a width in a $boxSizes value was not an int |
||
515 | */ |
||
516 | public function resizeMultiNonIntWidth() |
||
520 | |||
521 | /** |
||
522 | * @test |
||
523 | * @covers ::resizeMulti |
||
524 | * @expectedException \InvalidArgumentException |
||
525 | * @expectedExceptionMessage a height in a $boxSizes value was not an int |
||
526 | */ |
||
527 | public function resizeMultiNonIntHeight() |
||
531 | |||
532 | /** |
||
533 | * @test |
||
534 | * @covers ::resize |
||
535 | */ |
||
536 | View Code Duplication | public function resizeTransparentImageWithTransparentBackground() |
|
543 | |||
544 | /** |
||
545 | * @test |
||
546 | * @covers ::resize |
||
547 | */ |
||
548 | View Code Duplication | public function resizeTransparentImageWithColorBackground() |
|
555 | |||
556 | /** |
||
557 | * @test |
||
558 | * @covers ::resize |
||
559 | */ |
||
560 | View Code Duplication | public function resizeTransparentImageWithUpsize() |
|
567 | |||
568 | /** |
||
569 | * @test |
||
570 | * @covers ::resize |
||
571 | */ |
||
572 | View Code Duplication | public function resizeTransparentImageWithColorOfBlur() |
|
579 | |||
580 | /** |
||
581 | * @test |
||
582 | * @covers ::resize |
||
583 | */ |
||
584 | View Code Duplication | public function resizeTransparentImageWithBlurBackground() |
|
591 | |||
592 | /** |
||
593 | * @test |
||
594 | * @covers ::write |
||
595 | */ |
||
596 | public function write() |
||
620 | |||
621 | /** |
||
622 | * @test |
||
623 | * @covers ::write |
||
624 | * @expectedException \InvalidArgumentException |
||
625 | * @expectedExceptionMessage $options["directoryMode"] was not an int |
||
626 | */ |
||
627 | public function writeNonIntDirectoryMode() |
||
631 | |||
632 | /** |
||
633 | * @test |
||
634 | * @covers ::write |
||
635 | * @expectedException \InvalidArgumentException |
||
636 | * @expectedExceptionMessage $options["fileMode"] was not an int |
||
637 | */ |
||
638 | public function writeNonIntFileMode() |
||
642 | |||
643 | /** |
||
644 | * @test |
||
645 | * @covers ::write |
||
646 | * @expectedException \InvalidArgumentException |
||
647 | * @expectedExceptionMessage $options["format"] was not a string |
||
648 | */ |
||
649 | public function writeNonStringFormat() |
||
653 | |||
654 | /** |
||
655 | * @test |
||
656 | * @covers ::write |
||
657 | * @expectedException \InvalidArgumentException |
||
658 | * @expectedExceptionMessage $options["stripHeaders"] was not a bool |
||
659 | */ |
||
660 | public function writeNonBoolStripHeaders() |
||
664 | |||
665 | /** |
||
666 | * Verify that stripHeaders strips exif headers. |
||
667 | * |
||
668 | * @test |
||
669 | * @covers ::stripHeaders |
||
670 | */ |
||
671 | public function stripHeaders() |
||
683 | |||
684 | /** |
||
685 | * Verify that stripHeaders fails with a missing image. |
||
686 | * |
||
687 | * @test |
||
688 | * @covers ::stripHeaders |
||
689 | * @expectedException \ImagickException |
||
690 | */ |
||
691 | public function stripHeadersMissingImage() |
||
695 | |||
696 | private function assertSameImage(\Imagick $expected, \Imagick $actual) |
||
701 | |||
702 | private function getTestImage(string $filename) : \Imagick |
||
708 | } |
||
709 |
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.