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 DNDataArchive 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 DNDataArchive, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
50 | class DNDataArchive extends DataObject { |
||
51 | |||
52 | private static $db = array( |
||
53 | 'UploadToken' => 'Varchar(8)', |
||
54 | 'ArchiveFileHash' => 'Varchar(32)', |
||
55 | "Mode" => "Enum('all, assets, db', '')", |
||
56 | "IsBackup" => "Boolean", |
||
57 | "IsManualUpload" => "Boolean", |
||
58 | ); |
||
59 | |||
60 | private static $has_one = array( |
||
61 | 'Author' => 'Member', |
||
62 | 'OriginalEnvironment' => 'DNEnvironment', |
||
63 | 'Environment' => 'DNEnvironment', |
||
64 | 'ArchiveFile' => 'File' |
||
65 | ); |
||
66 | |||
67 | private static $has_many = array( |
||
68 | 'DataTransfers' => 'DNDataTransfer', |
||
69 | ); |
||
70 | |||
71 | private static $singular_name = 'Data Archive'; |
||
72 | |||
73 | private static $plural_name = 'Data Archives'; |
||
74 | |||
75 | private static $summary_fields = array( |
||
76 | 'Created' => 'Created', |
||
77 | 'Author.Title' => 'Author', |
||
78 | 'Environment.Project.Name' => 'Project', |
||
79 | 'OriginalEnvironment.Name' => 'Origin', |
||
80 | 'Environment.Name' => 'Environment', |
||
81 | 'ArchiveFile.Name' => 'File', |
||
82 | ); |
||
83 | |||
84 | private static $searchable_fields = array( |
||
85 | 'Environment.Project.Name' => array( |
||
86 | 'title' => 'Project', |
||
87 | ), |
||
88 | 'OriginalEnvironment.Name' => array( |
||
89 | 'title' => 'Origin', |
||
90 | ), |
||
91 | 'Environment.Name' => array( |
||
92 | 'title' => 'Environment', |
||
93 | ), |
||
94 | 'UploadToken' => array( |
||
95 | 'title' => 'Upload Token', |
||
96 | ), |
||
97 | 'Mode' => array( |
||
98 | 'title' => 'Mode', |
||
99 | ), |
||
100 | ); |
||
101 | |||
102 | private static $_cache_can_restore = array(); |
||
103 | |||
104 | private static $_cache_can_download = array(); |
||
105 | |||
106 | public static function get_mode_map() { |
||
107 | return array( |
||
108 | 'all' => 'Database and Assets', |
||
109 | 'db' => 'Database only', |
||
110 | 'assets' => 'Assets only', |
||
111 | ); |
||
112 | } |
||
113 | |||
114 | /** |
||
115 | * Returns a unique token to correlate an offline item (posted DVD) |
||
116 | * with a specific archive placeholder. |
||
117 | * |
||
118 | * @return string |
||
119 | */ |
||
120 | public static function generate_upload_token($chars = 8) { |
||
124 | |||
125 | public function onBeforeWrite() { |
||
126 | if(!$this->AuthorID) { |
||
127 | $this->AuthorID = Member::currentUserID(); |
||
128 | } |
||
129 | |||
130 | parent::onBeforeWrite(); |
||
131 | } |
||
132 | |||
133 | public function onAfterDelete() { |
||
136 | |||
137 | public function getCMSFields() { |
||
138 | $fields = parent::getCMSFields(); |
||
139 | $fields->removeByName('OriginalEnvironmentID'); |
||
140 | $fields->removeByName('EnvironmentID'); |
||
141 | $fields->removeByName('ArchiveFile'); |
||
142 | $fields->addFieldsToTab( |
||
143 | 'Root.Main', |
||
144 | array( |
||
145 | new ReadonlyField('ProjectName', 'Project', $this->Environment()->Project()->Name), |
||
146 | new ReadonlyField('OriginalEnvironmentName', 'OriginalEnvironment', $this->OriginalEnvironment()->Name), |
||
147 | new ReadonlyField('EnvironmentName', 'Environment', $this->Environment()->Name), |
||
148 | $linkField = new ReadonlyField( |
||
149 | 'DataArchive', |
||
150 | 'Archive File', |
||
151 | sprintf( |
||
152 | '<a href="%s">%s</a>', |
||
153 | $this->ArchiveFile()->AbsoluteURL, |
||
154 | $this->ArchiveFile()->Filename |
||
155 | ) |
||
156 | ), |
||
157 | new GridField( |
||
158 | 'DataTransfers', |
||
159 | 'Transfers', |
||
160 | $this->DataTransfers() |
||
161 | ), |
||
162 | ) |
||
163 | ); |
||
164 | $linkField->dontEscape = true; |
||
165 | $fields = $fields->makeReadonly(); |
||
166 | |||
167 | return $fields; |
||
168 | } |
||
169 | |||
170 | public function getDefaultSearchContext() { |
||
171 | $context = parent::getDefaultSearchContext(); |
||
172 | $context->getFields()->dataFieldByName('Mode')->setHasEmptyDefault(true); |
||
173 | |||
174 | return $context; |
||
175 | } |
||
176 | |||
177 | /** |
||
178 | * Calculates and returns a human-readable size of this archive file. If the file exists, it will determine |
||
179 | * whether to display the output in bytes, kilobytes, megabytes, or gigabytes. |
||
180 | * |
||
181 | * @return string The human-readable size of this archive file |
||
182 | */ |
||
183 | public function FileSize() { |
||
184 | if($this->ArchiveFile()->exists()) { |
||
185 | return $this->ArchiveFile()->getSize(); |
||
186 | } else { |
||
187 | return "N/A"; |
||
188 | } |
||
189 | } |
||
190 | |||
191 | public function getModeNice() { |
||
192 | if($this->Mode == 'all') { |
||
193 | return 'database and assets'; |
||
194 | } else { |
||
195 | return $this->Mode; |
||
196 | } |
||
197 | } |
||
198 | |||
199 | /** |
||
200 | * Some archives don't have files attached to them yet, |
||
201 | * because a file has been posted offline and is waiting to be uploaded |
||
202 | * against this "archive placeholder". |
||
203 | * |
||
204 | * @return boolean |
||
205 | */ |
||
206 | public function isPending() { |
||
209 | |||
210 | /** |
||
211 | * Inferred from both restore and backup permissions. |
||
212 | * |
||
213 | * @param Member|null $member The {@link Member} object to test against. |
||
214 | */ |
||
215 | public function canView($member = null) { |
||
218 | |||
219 | /** |
||
220 | * Whether a {@link Member} can restore this archive to an environment. |
||
221 | * This only needs to be checked *once* per member and environment. |
||
222 | * |
||
223 | * @param Member|null $member The {@link Member} object to test against. |
||
224 | * @return true if $member (or the currently logged in member if null) can upload this archive |
||
225 | */ |
||
226 | View Code Duplication | public function canRestore($member = null) { |
|
|
|||
227 | $memberID = $member ? $member->ID : Member::currentUserID(); |
||
228 | if(!$memberID) { |
||
229 | return false; |
||
230 | } |
||
231 | |||
232 | $key = $memberID . '-' . $this->EnvironmentID; |
||
233 | if(!isset(self::$_cache_can_restore[$key])) { |
||
234 | self::$_cache_can_restore[$key] = $this->Environment()->canUploadArchive($member); |
||
235 | } |
||
236 | |||
237 | return self::$_cache_can_restore[$key]; |
||
238 | } |
||
239 | |||
240 | /** |
||
241 | * Whether a {@link Member} can download this archive to their PC. |
||
242 | * This only needs to be checked *once* per member and environment. |
||
243 | * |
||
244 | * @param Member|null $member The {@link Member} object to test against. |
||
245 | * @return true if $member (or the currently logged in member if null) can download this archive |
||
246 | */ |
||
247 | View Code Duplication | public function canDownload($member = null) { |
|
248 | $memberID = $member ? $member->ID : Member::currentUserID(); |
||
249 | if(!$memberID) { |
||
250 | return false; |
||
251 | } |
||
252 | |||
253 | $key = $memberID . '-' . $this->EnvironmentID; |
||
254 | if(!isset(self::$_cache_can_download[$key])) { |
||
255 | self::$_cache_can_download[$key] = $this->Environment()->canDownloadArchive($member); |
||
256 | } |
||
257 | return self::$_cache_can_download[$key]; |
||
258 | } |
||
259 | |||
260 | /** |
||
261 | * Whether a {@link Member} can delete this archive from staging area. |
||
262 | * |
||
263 | * @param Member|null $member The {@link Member} object to test against. |
||
264 | * @return boolean if $member (or the currently logged in member if null) can delete this archive |
||
265 | */ |
||
266 | public function canDelete($member = null) { |
||
269 | |||
270 | /** |
||
271 | * Check if this member can move archive into the environment. |
||
272 | * |
||
273 | * @param DNEnvironment $targetEnv Environment to check. |
||
274 | * @param Member|null $member The {@link Member} object to test against. If null, uses Member::currentMember(); |
||
275 | * |
||
276 | * @return boolean true if $member can upload archives linked to this environment, false if they can't. |
||
277 | */ |
||
278 | public function canMoveTo($targetEnv, $member = null) { |
||
308 | |||
309 | /** |
||
310 | * Finds all environments within this project where the archive can be moved to. |
||
311 | * Excludes current environment automatically. |
||
312 | * |
||
313 | * @return ArrayList List of valid environments. |
||
314 | */ |
||
315 | public function validTargetEnvironments() { |
||
324 | |||
325 | /** |
||
326 | * Returns a unique filename, including project/environment/timestamp details. |
||
327 | * @return string |
||
328 | */ |
||
329 | public function generateFilename(DNDataTransfer $dataTransfer) { |
||
342 | |||
343 | /** |
||
344 | * Returns a path unique to a specific transfer, including project/environment details. |
||
345 | * Does not create the path on the filesystem. Can be used to store files related to this transfer. |
||
346 | * |
||
347 | * @param DNDataTransfer |
||
348 | * @return string Absolute file path |
||
349 | */ |
||
350 | public function generateFilepath(DNDataTransfer $dataTransfer) { |
||
362 | |||
363 | /** |
||
364 | * Attach an sspak file path to this archive and associate the transfer. |
||
365 | * Does the job of creating a {@link File} record, and setting correct paths into the assets directory. |
||
366 | * |
||
367 | * @param string $sspakFilepath |
||
368 | * @param DNDataTransfer $dataTransfer |
||
369 | * @return bool |
||
370 | */ |
||
371 | public function attachFile($sspakFilepath, DNDataTransfer $dataTransfer) { |
||
405 | |||
406 | /** |
||
407 | * Extract the current sspak contents into the given working directory. |
||
408 | * This also extracts the assets and database and puts them into |
||
409 | * <workingdir>/database.sql and <workingdir>/assets, respectively. |
||
410 | * |
||
411 | * @param string|null $workingDir The path to extract to |
||
412 | * @throws RuntimeException |
||
413 | * @return bool |
||
414 | */ |
||
415 | public function extractArchive($workingDir = null) { |
||
463 | |||
464 | /** |
||
465 | * Validate that an sspak contains the correct content. |
||
466 | * |
||
467 | * For example, if the user uploaded an sspak containing just the db, but declared in the form |
||
468 | * that it contained db+assets, then the archive is not valid. |
||
469 | * |
||
470 | * @param string|null $mode "db", "assets", or "all". This is the content we're checking for. Default to the archive setting |
||
471 | * @return ValidationResult |
||
472 | */ |
||
473 | public function validateArchiveContents($mode = null) { |
||
506 | |||
507 | /** |
||
508 | * Given a path that already exists and contains an extracted sspak, including |
||
509 | * the assets, fix all of the file permissions so they're in a state ready to |
||
510 | * be pushed to remote servers. |
||
511 | * |
||
512 | * Normally, command line tar will use permissions found in the archive, but will |
||
513 | * substract the user's umask from them. This has a potential to create unreadable |
||
514 | * files, e.g. cygwin on Windows will pack files with mode 000, hence why this fix |
||
515 | * is necessary. |
||
516 | * |
||
517 | * @param string|null $workingDir The path of where the sspak has been extracted to |
||
518 | * @throws RuntimeException |
||
519 | * @return bool |
||
520 | */ |
||
521 | public function fixArchivePermissions($workingDir) { |
||
540 | |||
541 | /** |
||
542 | * Given extracted sspak contents, create an sspak from it |
||
543 | * and overwrite the current ArchiveFile with it's contents. |
||
544 | * |
||
545 | * @param string|null $workingDir The path of where the sspak has been extracted to |
||
546 | * @return bool |
||
547 | */ |
||
548 | public function setArchiveFromFiles($workingDir) { |
||
570 | |||
571 | } |
||
572 |
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.