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 TidypicsAlbum 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 TidypicsAlbum, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
11 | class TidypicsAlbum extends ElggObject { |
||
12 | /** |
||
13 | * Sets the internal attributes |
||
14 | */ |
||
15 | protected function initializeAttributes() { |
||
20 | |||
21 | /** |
||
22 | * Constructor |
||
23 | * @param mixed $guid |
||
24 | */ |
||
25 | public function __construct($guid = null) { |
||
28 | |||
29 | /** |
||
30 | * Save an album |
||
31 | * |
||
32 | * @return bool |
||
33 | */ |
||
34 | public function save() { |
||
52 | |||
53 | /** |
||
54 | * Delete album |
||
55 | * |
||
56 | * @return bool |
||
57 | */ |
||
58 | public function delete() { |
||
65 | |||
66 | /** |
||
67 | * Get the title of the photo album |
||
68 | * |
||
69 | * @return string |
||
70 | */ |
||
71 | public function getTitle() { |
||
74 | |||
75 | /** |
||
76 | * Get the URL for this album |
||
77 | * |
||
78 | * @return string |
||
79 | */ |
||
80 | public function getURL() { |
||
85 | |||
86 | /** |
||
87 | * Get an array of image objects |
||
88 | * |
||
89 | * @param int $limit |
||
90 | * @param int $offset |
||
91 | * @return array |
||
92 | */ |
||
93 | public function getImages($limit, $offset = 0) { |
||
107 | |||
108 | /** |
||
109 | * View a list of images |
||
110 | * |
||
111 | * @param array $options Options to pass to elgg_view_entity_list() |
||
112 | * @return string |
||
113 | */ |
||
114 | public function viewImages(array $options = array()) { |
||
141 | |||
142 | /** |
||
143 | * Returns the cover image entity |
||
144 | * @return TidypicsImage |
||
145 | */ |
||
146 | public function getCoverImage() { |
||
149 | |||
150 | /** |
||
151 | * Get the GUID of the album cover |
||
152 | * |
||
153 | * @return int |
||
154 | */ |
||
155 | public function getCoverImageGuid() { |
||
170 | |||
171 | /** |
||
172 | * Set the GUID for the album cover |
||
173 | * |
||
174 | * @param int $guid |
||
175 | * @return bool |
||
176 | */ |
||
177 | public function setCoverImageGuid($guid) { |
||
185 | |||
186 | /** |
||
187 | * Get the number of photos in the album |
||
188 | * |
||
189 | * @return int |
||
190 | */ |
||
191 | public function getSize() { |
||
194 | |||
195 | /** |
||
196 | * Returns an order list of image guids |
||
197 | * |
||
198 | * @return array |
||
199 | */ |
||
200 | public function getImageList() { |
||
225 | |||
226 | /** |
||
227 | * Sets the album image order |
||
228 | * |
||
229 | * @param array $list An indexed array of image guids |
||
230 | * @return bool |
||
231 | */ |
||
232 | public function setImageList($list) { |
||
244 | |||
245 | /** |
||
246 | * Add new images to the front of the image list |
||
247 | * |
||
248 | * @param array $list An indexed array of image guids |
||
249 | * @return bool |
||
250 | */ |
||
251 | public function prependImageList($list) { |
||
256 | |||
257 | /** |
||
258 | * Get the previous image in the album. Wraps around to the last image if given the first. |
||
259 | * |
||
260 | * @param int $guid GUID of the current image |
||
261 | * @return TidypicsImage |
||
262 | */ |
||
263 | View Code Duplication | public function getPreviousImage($guid) { |
|
275 | |||
276 | /** |
||
277 | * Get the next image in the album. Wraps around to the first image if given the last. |
||
278 | * |
||
279 | * @param int $guid GUID of the current image |
||
280 | * @return TidypicsImage |
||
281 | */ |
||
282 | View Code Duplication | public function getNextImage($guid) { |
|
294 | |||
295 | /** |
||
296 | * Get the index into the album for a particular image |
||
297 | * |
||
298 | * @param int $guid GUID of the image |
||
299 | * @return int |
||
300 | */ |
||
301 | public function getIndex($guid) { |
||
304 | |||
305 | /** |
||
306 | * Remove an image from the album list |
||
307 | * |
||
308 | * @param int $imageGuid |
||
309 | * @return bool |
||
310 | */ |
||
311 | public function removeImage($imageGuid) { |
||
323 | |||
324 | /** |
||
325 | * Has enough time elapsed between the last_notified and notify_interval setting? |
||
326 | * |
||
327 | * @return bool |
||
328 | */ |
||
329 | public function shouldNotify() { |
||
332 | |||
333 | /** |
||
334 | * Delete all the images in this album |
||
335 | */ |
||
336 | protected function deleteImages() { |
||
358 | |||
359 | /** |
||
360 | * Delete the album directory on disk |
||
361 | */ |
||
362 | protected function deleteAlbumDir() { |
||
394 | } |
||
395 |
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.