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 CImageResizer 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 CImageResizer, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
9 | class CImageResizer |
||
10 | { |
||
11 | /** |
||
12 | * Log function. |
||
13 | */ |
||
14 | private $log; |
||
15 | |||
16 | |||
17 | |||
18 | /** |
||
19 | * Source image dimensions, calculated from loaded image. |
||
20 | */ |
||
21 | private $srcWidth; |
||
22 | private $srcHeight; |
||
23 | |||
24 | |||
25 | |||
26 | /** |
||
27 | * Set as expected target image/canvas dimensions. |
||
28 | */ |
||
29 | private $targetWidth; |
||
30 | private $targetHeight; |
||
31 | |||
32 | |||
33 | |||
34 | /** |
||
35 | * Where should the image go on the canvas. |
||
36 | */ |
||
37 | private $destinationX; |
||
38 | private $destinationY; |
||
39 | private $destinationWidth; |
||
40 | private $destinationHeight; |
||
41 | |||
42 | |||
43 | |||
44 | /** |
||
45 | * Which parts to crop from the source. |
||
46 | */ |
||
47 | private $cropX; |
||
48 | private $cropY; |
||
49 | private $cropWidth; |
||
50 | private $cropHeight; |
||
51 | |||
52 | |||
53 | |||
54 | /** |
||
55 | * Change target height & width when different dpr, dpr 2 means double |
||
56 | * image dimensions. |
||
57 | */ |
||
58 | private $dpr = null; |
||
59 | |||
60 | |||
61 | |||
62 | /** |
||
63 | * Set aspect ratio for the target image. |
||
64 | */ |
||
65 | private $aspectRatio; |
||
66 | |||
67 | |||
68 | |||
69 | /** |
||
70 | * Array with details on how to crop. |
||
71 | * Array contains xxxxxx |
||
72 | */ |
||
73 | public $crop; |
||
74 | public $cropOrig; // Save original value? |
||
75 | |||
76 | |||
77 | |||
78 | /** |
||
79 | * Area to use for target image, crop out parts not in area. |
||
80 | * Array with top, right, bottom, left percentage values to crop out. |
||
81 | */ |
||
82 | private $area; |
||
83 | |||
84 | |||
85 | |||
86 | /** |
||
87 | * Pixel offset in source image to decide which part of image is used. |
||
88 | * Array with top, right, bottom, left percentage values to crop out. |
||
89 | */ |
||
90 | private $offset; |
||
91 | |||
92 | |||
93 | |||
94 | /** |
||
95 | * Resize strategy, image should keep its original ratio. |
||
96 | */ |
||
97 | const KEEP_RATIO = 1; |
||
98 | |||
99 | |||
100 | |||
101 | /** |
||
102 | * Resize strategy, image should crop and fill area. |
||
103 | */ |
||
104 | const CROP_TO_FIT = 2; |
||
105 | |||
106 | |||
107 | |||
108 | /** |
||
109 | * Resize strategy, image should fit in area and fill remains. |
||
110 | */ |
||
111 | const FILL_TO_FIT = 3; |
||
112 | |||
113 | |||
114 | |||
115 | /** |
||
116 | * Resize strategy, image should stretch to fit in area. |
||
117 | */ |
||
118 | const STRETCH = 4; |
||
119 | |||
120 | |||
121 | |||
122 | /** |
||
123 | * The currently selected resize strategy. |
||
124 | */ |
||
125 | private $resizeStrategy = self::KEEP_RATIO; |
||
126 | |||
127 | |||
128 | |||
129 | /** |
||
130 | * Allow upscale of smaller images by default, set to false to disallow. |
||
131 | */ |
||
132 | private $upscale = true; |
||
133 | |||
134 | |||
135 | |||
136 | /** |
||
137 | * Constructor, set log function to use for verbose logging or null |
||
138 | * to disable logging. |
||
139 | * |
||
140 | * @param callable $log function to call for logging. |
||
141 | */ |
||
142 | 113 | public function __construct($log = null) |
|
146 | |||
147 | |||
148 | |||
149 | /** |
||
150 | * Log string using logger. |
||
151 | * |
||
152 | * @param string $str to log. |
||
153 | */ |
||
154 | 104 | public function log($str) |
|
160 | |||
161 | |||
162 | |||
163 | /** |
||
164 | * Set source dimensions. |
||
165 | * |
||
166 | * @param integer $width of source image. |
||
167 | * @param integer $height of source image. |
||
168 | * |
||
169 | * @throws Exception |
||
170 | * |
||
171 | * @return $this |
||
172 | */ |
||
173 | 94 | public function setSource($width, $height) |
|
181 | |||
182 | |||
183 | |||
184 | /** |
||
185 | * Get resize strategy as string. |
||
186 | * |
||
187 | * @return string |
||
188 | */ |
||
189 | 70 | public function getResizeStrategyAsString() |
|
212 | |||
213 | |||
214 | |||
215 | /** |
||
216 | * Set resize strategy as KEEP_RATIO, CROP_TO_FIT or FILL_TO_FIT. |
||
217 | * |
||
218 | * @param integer $strategy |
||
219 | * |
||
220 | * @return $this |
||
221 | */ |
||
222 | 70 | public function setResizeStrategy($strategy) |
|
229 | |||
230 | |||
231 | |||
232 | /** |
||
233 | * Allow or disallow upscale smaller images. |
||
234 | * |
||
235 | * @param boolean $upscale |
||
236 | * |
||
237 | * @return $this |
||
238 | */ |
||
239 | 22 | public function allowUpscale($upscale) |
|
246 | |||
247 | |||
248 | |||
249 | /** |
||
250 | * Check if a value is upscaled or not by compare $current to $orig, |
||
251 | * if $current is larger than $orig then check if upscaled is allowed. |
||
252 | * |
||
253 | * @param float &$current value to check and change. |
||
254 | * @param float $orig value to check against. |
||
255 | * |
||
256 | * @return float as the value respected by the upscale setting. |
||
257 | */ |
||
258 | 40 | public function respectUpscale(&$current, $orig) |
|
265 | |||
266 | |||
267 | |||
268 | /** |
||
269 | * Set base for requested width and height. |
||
270 | * |
||
271 | * @param numeric|null $width as requested target width |
||
272 | * @param numeric|null $height as requested target height |
||
273 | * |
||
274 | * @throws Exception |
||
275 | * |
||
276 | * @return $this |
||
277 | */ |
||
278 | 98 | public function setBaseWidthHeight($width = null, $height = null) |
|
309 | |||
310 | |||
311 | |||
312 | /** |
||
313 | * Set base for requested aspect ratio. |
||
314 | * |
||
315 | * @param float|null $aspectRatio as requested aspect ratio |
||
316 | * |
||
317 | * @throws Exception |
||
318 | * |
||
319 | * @return $this |
||
320 | */ |
||
321 | 15 | View Code Duplication | public function setBaseAspecRatio($aspectRatio = null) |
335 | |||
336 | |||
337 | |||
338 | /** |
||
339 | * Set base for requested device pixel ratio. |
||
340 | * |
||
341 | * @param float $dpr as requested density pixel rate |
||
342 | * |
||
343 | * @throws Exception |
||
344 | * |
||
345 | * @return $this |
||
346 | */ |
||
347 | 14 | View Code Duplication | public function setBaseDevicePixelRate($dpr = null) |
361 | |||
362 | |||
363 | |||
364 | /** |
||
365 | * Calculate target width and height by considering the selected |
||
366 | * aspect ratio. |
||
367 | * |
||
368 | * @throws Exception |
||
369 | * |
||
370 | * @return $this |
||
371 | */ |
||
372 | 26 | public function prepareByConsiderAspectRatio() |
|
422 | |||
423 | |||
424 | |||
425 | /** |
||
426 | * Calculate target width and height by considering the selected |
||
427 | * dpr. |
||
428 | * |
||
429 | * @throws Exception |
||
430 | * |
||
431 | * @return $this |
||
432 | */ |
||
433 | 26 | public function prepareByConsiderDpr() |
|
459 | |||
460 | |||
461 | |||
462 | /** |
||
463 | * Calculate target width and height and do sanity checks on constraints. |
||
464 | * After this method the $targetWidth and $targetHeight will have |
||
465 | * the expected dimensions on the target image. |
||
466 | * |
||
467 | * @throws Exception |
||
468 | * |
||
469 | * @return $this |
||
470 | */ |
||
471 | 26 | public function prepareTargetDimensions() |
|
482 | |||
483 | |||
484 | |||
485 | /** |
||
486 | * Calculate new width and height of image. |
||
487 | * |
||
488 | * @return $this |
||
489 | */ |
||
490 | 65 | public function calculateTargetWidthAndHeight() |
|
678 | |||
679 | |||
680 | |||
681 | /** |
||
682 | * Get source width. |
||
683 | * |
||
684 | * @return integer as source width |
||
685 | */ |
||
686 | 1 | public function getSourceWidth() |
|
690 | |||
691 | |||
692 | |||
693 | /** |
||
694 | * Get source height. |
||
695 | * |
||
696 | * @return integer as source height |
||
697 | */ |
||
698 | 1 | public function getSourceHeight() |
|
702 | |||
703 | |||
704 | |||
705 | /** |
||
706 | * Get target width. |
||
707 | * |
||
708 | * @return integer as target width |
||
709 | */ |
||
710 | 93 | public function getTargetWidth() |
|
714 | |||
715 | |||
716 | |||
717 | /** |
||
718 | * Get target height. |
||
719 | * |
||
720 | * @return integer as target height |
||
721 | */ |
||
722 | 93 | public function getTargetHeight() |
|
726 | |||
727 | |||
728 | |||
729 | /** |
||
730 | * Get destination x. |
||
731 | * |
||
732 | * @return integer as destination x |
||
733 | */ |
||
734 | 16 | public function getDestinationX() |
|
738 | |||
739 | |||
740 | |||
741 | /** |
||
742 | * Get destination y. |
||
743 | * |
||
744 | * @return integer as destination y |
||
745 | */ |
||
746 | 16 | public function getDestinationY() |
|
750 | |||
751 | |||
752 | |||
753 | /** |
||
754 | * Get destination width. |
||
755 | * |
||
756 | * @return integer as destination width |
||
757 | */ |
||
758 | 16 | public function getDestinationWidth() |
|
762 | |||
763 | |||
764 | |||
765 | /** |
||
766 | * Get destination height. |
||
767 | * |
||
768 | * @return integer as destination height |
||
769 | */ |
||
770 | 16 | public function getDestinationHeight() |
|
774 | |||
775 | |||
776 | |||
777 | /** |
||
778 | * Get crop position x. |
||
779 | * |
||
780 | * @return integer |
||
781 | */ |
||
782 | 42 | public function getCropX() |
|
786 | |||
787 | |||
788 | |||
789 | /** |
||
790 | * Get crop position y. |
||
791 | * |
||
792 | * @return integer |
||
793 | */ |
||
794 | 42 | public function getCropY() |
|
798 | |||
799 | |||
800 | |||
801 | /** |
||
802 | * Get crop width. |
||
803 | * |
||
804 | * @return integer |
||
805 | */ |
||
806 | 42 | public function getCropWidth() |
|
810 | |||
811 | |||
812 | |||
813 | /** |
||
814 | * Get crop height. |
||
815 | * |
||
816 | * @return integer |
||
817 | */ |
||
818 | 42 | public function getCropHeight() |
|
822 | } |
||
823 |
The break statement is not necessary if it is preceded for example by a return statement:
If you would like to keep this construct to be consistent with other case statements, you can safely mark this issue as a false-positive.