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 ImageProperty 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 ImageProperty, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
23 | class ImageProperty extends FileProperty |
||
24 | { |
||
25 | const DEFAULT_DRIVER_TYPE = 'imagick'; |
||
26 | |||
27 | const EFFECTS_EVENT_SAVE = 'save'; |
||
28 | const EFFECTS_EVENT_NEVER = 'never'; |
||
29 | const EFFECTS_EVENT_UPLOAD = 'upload'; |
||
30 | const DEFAULT_APPLY_EFFECTS = self::EFFECTS_EVENT_SAVE; |
||
31 | |||
32 | /** |
||
33 | * One or more effects to apply on the image. |
||
34 | * |
||
35 | * @var array |
||
36 | */ |
||
37 | private $effects = []; |
||
38 | |||
39 | /** |
||
40 | * Whether to apply any effects on the uploaded image. |
||
41 | * |
||
42 | * @var mixed |
||
43 | */ |
||
44 | private $applyEffects = self::DEFAULT_APPLY_EFFECTS; |
||
45 | |||
46 | /** |
||
47 | * The type of image processing engine. |
||
48 | * |
||
49 | * @var string |
||
50 | */ |
||
51 | private $driverType = self::DEFAULT_DRIVER_TYPE; |
||
52 | |||
53 | /** |
||
54 | * Internal storage of the image factory instance. |
||
55 | * |
||
56 | * @var ImageFactory |
||
57 | */ |
||
58 | private $imageFactory; |
||
59 | |||
60 | /** |
||
61 | * @return string |
||
62 | */ |
||
63 | public function type() |
||
64 | { |
||
65 | return 'image'; |
||
66 | } |
||
67 | |||
68 | /** |
||
69 | * Retrieve the image factory. |
||
70 | * |
||
71 | * @return ImageFactory |
||
72 | */ |
||
73 | public function imageFactory() |
||
74 | { |
||
75 | if ($this->imageFactory === null) { |
||
76 | $this->imageFactory = $this->createImageFactory(); |
||
77 | } |
||
78 | |||
79 | return $this->imageFactory; |
||
80 | } |
||
81 | |||
82 | /** |
||
83 | * Set the name of the property's image processing driver. |
||
84 | * |
||
85 | * @param string $type The processing engine. |
||
86 | * @throws InvalidArgumentException If the drive type is not a string. |
||
87 | * @return ImageProperty Chainable |
||
88 | */ |
||
89 | View Code Duplication | public function setDriverType($type) |
|
|
|||
90 | { |
||
91 | if (!is_string($type)) { |
||
92 | throw new InvalidArgumentException(sprintf( |
||
93 | 'Image driver type must be a string, received %s', |
||
94 | (is_object($type) ? get_class($type) : gettype($type)) |
||
95 | )); |
||
96 | } |
||
97 | |||
98 | $this->driverType = $type; |
||
99 | |||
100 | return $this; |
||
101 | } |
||
102 | |||
103 | /** |
||
104 | * Retrieve the name of the property's image processing driver. |
||
105 | * |
||
106 | * @return string |
||
107 | */ |
||
108 | public function getDriverType() |
||
109 | { |
||
110 | return $this->driverType; |
||
111 | } |
||
112 | |||
113 | /** |
||
114 | * Set whether effects should be applied. |
||
115 | * |
||
116 | * @param mixed $event When to apply affects. |
||
117 | * @throws OutOfBoundsException If the effects event does not exist. |
||
118 | * @return ImageProperty Chainable |
||
119 | */ |
||
120 | public function setApplyEffects($event) |
||
121 | { |
||
122 | if ($event === false) { |
||
123 | $this->applyEffects = self::EFFECTS_EVENT_NEVER; |
||
124 | return $this; |
||
125 | } |
||
126 | |||
127 | if ($event === null || $event === '') { |
||
128 | $this->applyEffects = self::EFFECTS_EVENT_SAVE; |
||
129 | return $this; |
||
130 | } |
||
131 | |||
132 | View Code Duplication | if (!in_array($event, $this->acceptedEffectsEvents())) { |
|
133 | if (!is_string($event)) { |
||
134 | $event = (is_object($event) ? get_class($event) : gettype($event)); |
||
135 | } |
||
136 | throw new OutOfBoundsException(sprintf( |
||
137 | 'Unsupported image property event "%s" provided', |
||
138 | $event |
||
139 | )); |
||
140 | } |
||
141 | |||
142 | $this->applyEffects = $event; |
||
143 | |||
144 | return $this; |
||
145 | } |
||
146 | |||
147 | /** |
||
148 | * Determine if effects should be applied. |
||
149 | * |
||
150 | * @return string Returns the property's condition on effects. |
||
151 | */ |
||
152 | public function getApplyEffects() |
||
153 | { |
||
154 | return $this->applyEffects; |
||
155 | } |
||
156 | |||
157 | /** |
||
158 | * Determine if effects should be applied. |
||
159 | * |
||
160 | * @param string|boolean $event A specific event to check or a global flag to set. |
||
161 | * @throws OutOfBoundsException If the effects event does not exist. |
||
162 | * @return mixed Returns TRUE or FALSE if the property applies effects for the given event. |
||
163 | */ |
||
164 | public function canApplyEffects($event) |
||
165 | { |
||
166 | View Code Duplication | if (!in_array($event, $this->acceptedEffectsEvents())) { |
|
167 | if (!is_string($event)) { |
||
168 | $event = (is_object($event) ? get_class($event) : gettype($event)); |
||
169 | } |
||
170 | throw new OutOfBoundsException(sprintf( |
||
171 | 'Unsupported image property event "%s" provided', |
||
172 | $event |
||
173 | )); |
||
174 | } |
||
175 | |||
176 | return $this->applyEffects === $event; |
||
177 | } |
||
178 | |||
179 | /** |
||
180 | * Retrieve the supported events where effects can be applied. |
||
181 | * |
||
182 | * @return array |
||
183 | */ |
||
184 | public function acceptedEffectsEvents() |
||
185 | { |
||
186 | return [ |
||
187 | self::EFFECTS_EVENT_UPLOAD, |
||
188 | self::EFFECTS_EVENT_SAVE, |
||
189 | self::EFFECTS_EVENT_NEVER, |
||
190 | ]; |
||
191 | } |
||
192 | |||
193 | /** |
||
194 | * Set (reset, in fact) the image effects. |
||
195 | * |
||
196 | * @param array $effects The effects to set to the image. |
||
197 | * @return ImageProperty Chainable |
||
198 | */ |
||
199 | public function setEffects(array $effects) |
||
200 | { |
||
201 | $this->effects = []; |
||
202 | foreach ($effects as $effect) { |
||
203 | $this->addEffect($effect); |
||
204 | } |
||
205 | return $this; |
||
206 | } |
||
207 | |||
208 | /** |
||
209 | * @param mixed $effect An image effect. |
||
210 | * @return ImageProperty Chainable |
||
211 | */ |
||
212 | public function addEffect($effect) |
||
213 | { |
||
214 | $this->effects[] = $effect; |
||
215 | return $this; |
||
216 | } |
||
217 | |||
218 | /** |
||
219 | * @return array |
||
220 | */ |
||
221 | public function getEffects() |
||
222 | { |
||
223 | return $this->effects; |
||
224 | } |
||
225 | |||
226 | /** |
||
227 | * Process the property's effects on the given image(s). |
||
228 | * |
||
229 | * @param mixed $value The target(s) to apply effects on. |
||
230 | * @param array $effects The effects to apply on the target. |
||
231 | * @param ImageInterface|null $image Optional. The image for processing. |
||
232 | * @return mixed Returns the given images. Depending on the effects applied, |
||
233 | * certain images might be renamed. |
||
234 | */ |
||
235 | public function processEffects($value, array $effects = null, ImageInterface $image = null) |
||
263 | |||
264 | /** |
||
265 | * Retrieves the default list of acceptable MIME types for uploaded files. |
||
266 | * |
||
267 | * This method should be overriden. |
||
268 | * |
||
269 | * @return string[] |
||
270 | */ |
||
271 | View Code Duplication | public function getDefaultAcceptedMimetypes() |
|
272 | { |
||
273 | return [ |
||
274 | 'image/gif', |
||
283 | |||
284 | /** |
||
285 | * Resolve the file extension from the given MIME type. |
||
286 | * |
||
287 | * @param string $type The MIME type to resolve. |
||
288 | * @return string|null The extension based on the MIME type. |
||
289 | */ |
||
290 | View Code Duplication | protected function resolveExtensionFromMimeType($type) |
|
313 | |||
314 | /** |
||
315 | * @param mixed $val The value, at time of saving. |
||
316 | * @return mixed |
||
317 | */ |
||
318 | public function save($val) |
||
328 | |||
329 | /** |
||
330 | * Apply effects to the uploaded data URI(s). |
||
331 | * |
||
332 | * @see FileProperty::fileUpload() |
||
333 | * @param string $fileData The file data, raw. |
||
334 | * @return string |
||
335 | */ |
||
336 | View Code Duplication | public function dataUpload($fileData) |
|
346 | |||
347 | /** |
||
348 | * Apply effects to the uploaded file(s). |
||
349 | * |
||
350 | * @see FileProperty::fileUpload() |
||
351 | * @param array $fileData The file data to upload. |
||
352 | * @return string |
||
353 | */ |
||
354 | View Code Duplication | public function fileUpload(array $fileData) |
|
364 | |||
365 | /** |
||
366 | * Set an image factory. |
||
367 | * |
||
368 | * @param ImageFactory $factory The image factory, to manipulate images. |
||
369 | * @return self |
||
370 | */ |
||
371 | protected function setImageFactory(ImageFactory $factory) |
||
377 | |||
378 | /** |
||
379 | * Create an image factory. |
||
380 | * |
||
381 | * @return ImageFactory |
||
382 | */ |
||
383 | protected function createImageFactory() |
||
387 | |||
388 | /** |
||
389 | * Create an image. |
||
390 | * |
||
391 | * @return ImageInterface |
||
392 | */ |
||
393 | protected function createImage() |
||
397 | |||
398 | /** |
||
399 | * @return array |
||
400 | */ |
||
401 | protected function batchEffects() |
||
451 | |||
452 | /** |
||
453 | * Process the property's effects on the given image. |
||
454 | * |
||
455 | * @param string $value The target to apply effects on. |
||
456 | * @param array $effects The effects to apply on the target. |
||
457 | * @param ImageInterface|null $image Optional. The image for processing. |
||
458 | * @throws InvalidArgumentException If the $value is not a string. |
||
459 | * @return mixed Returns the processed target or NULL. |
||
460 | */ |
||
461 | private function processEffectsOne($value, array $effects = null, ImageInterface $image = null) |
||
585 | } |
||
586 |
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.