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 Migrator 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 Migrator, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
16 | class Migrator |
||
17 | { |
||
18 | /** |
||
19 | * Migrator configuration array. |
||
20 | * |
||
21 | * @var array |
||
22 | */ |
||
23 | protected $config; |
||
24 | |||
25 | /** |
||
26 | * Directory to store m. |
||
27 | * |
||
28 | * @var string |
||
29 | */ |
||
30 | protected $dir; |
||
31 | |||
32 | /** |
||
33 | * Directory to store archive m. |
||
34 | * |
||
35 | * @var string |
||
36 | */ |
||
37 | protected $dir_archive; |
||
38 | |||
39 | /** |
||
40 | * User transaction default. |
||
41 | * |
||
42 | * @var bool |
||
43 | */ |
||
44 | protected $use_transaction; |
||
45 | |||
46 | /** |
||
47 | * Files interactions. |
||
48 | * |
||
49 | * @var FileStorageInterface |
||
50 | */ |
||
51 | protected $files; |
||
52 | |||
53 | /** |
||
54 | * Interface that gives us access to the database. |
||
55 | * |
||
56 | * @var DatabaseStorageInterface |
||
57 | */ |
||
58 | protected $database; |
||
59 | |||
60 | /** |
||
61 | * TemplatesCollection instance. |
||
62 | * |
||
63 | * @var TemplatesCollection |
||
64 | */ |
||
65 | protected $templates; |
||
66 | |||
67 | /** |
||
68 | * Constructor. |
||
69 | * |
||
70 | * @param array $config |
||
71 | * @param TemplatesCollection $templates |
||
72 | * @param DatabaseStorageInterface $database |
||
73 | * @param FileStorageInterface $files |
||
74 | */ |
||
75 | public function __construct($config, TemplatesCollection $templates, DatabaseStorageInterface $database = null, FileStorageInterface $files = null) |
||
76 | { |
||
77 | $this->config = $config; |
||
78 | $this->dir = $config['dir']; |
||
79 | $this->dir_archive = isset($config['dir_archive']) ? $config['dir_archive'] : 'archive'; |
||
80 | $this->use_transaction = isset($config['use_transaction']) ? $config['use_transaction'] : false; |
||
81 | |||
82 | if (isset($config['default_fields']) && is_array($config['default_fields'])) { |
||
83 | foreach ($config['default_fields'] as $class => $default_fields) { |
||
84 | FieldConstructor::$defaultFields[$class] = $default_fields; |
||
85 | } |
||
86 | } |
||
87 | |||
88 | $this->templates = $templates; |
||
89 | $this->database = $database ?: new BitrixDatabaseStorage($config['table']); |
||
90 | $this->files = $files ?: new FileStorage(); |
||
91 | } |
||
92 | |||
93 | /** |
||
94 | * Create migration file. |
||
95 | * |
||
96 | * @param string $name - migration name |
||
97 | * @param string $templateName |
||
98 | * @param array $replace - array of placeholders that should be replaced with a given values. |
||
99 | * @param string $subDir |
||
100 | * |
||
101 | * @return string |
||
102 | */ |
||
103 | public function createMigration($name, $templateName, array $replace = [], $subDir = '') |
||
124 | |||
125 | /** |
||
126 | * Run all migrations that were not run before. |
||
127 | */ |
||
128 | public function runMigrations() |
||
144 | |||
145 | /** |
||
146 | * Run a given migration. |
||
147 | * |
||
148 | * @param string $file |
||
149 | * |
||
150 | * @throws Exception |
||
151 | * |
||
152 | * @return string |
||
153 | */ |
||
154 | View Code Duplication | public function runMigration($file) |
|
168 | |||
169 | /** |
||
170 | * Log successful migration. |
||
171 | * |
||
172 | * @param string $migration |
||
173 | * |
||
174 | * @return void |
||
175 | */ |
||
176 | public function logSuccessfulMigration($migration) |
||
180 | |||
181 | /** |
||
182 | * Get ran migrations. |
||
183 | * |
||
184 | * @return array |
||
185 | */ |
||
186 | public function getRanMigrations() |
||
190 | |||
191 | /** |
||
192 | * Get all migrations. |
||
193 | * |
||
194 | * @return array |
||
195 | */ |
||
196 | public function getAllMigrations() |
||
200 | |||
201 | /** |
||
202 | * Determine whether migration file for migration exists. |
||
203 | * |
||
204 | * @param string $migration |
||
205 | * |
||
206 | * @return bool |
||
207 | */ |
||
208 | public function doesMigrationFileExist($migration) |
||
212 | |||
213 | /** |
||
214 | * Rollback a given migration. |
||
215 | * |
||
216 | * @param string $file |
||
217 | * |
||
218 | * @throws Exception |
||
219 | * |
||
220 | * @return mixed |
||
221 | */ |
||
222 | View Code Duplication | public function rollbackMigration($file) |
|
234 | |||
235 | /** |
||
236 | * Remove a migration name from the database so it can be run again. |
||
237 | * |
||
238 | * @param string $file |
||
239 | * |
||
240 | * @return void |
||
241 | */ |
||
242 | public function removeSuccessfulMigrationFromLog($file) |
||
246 | |||
247 | /** |
||
248 | * Delete migration file. |
||
249 | * |
||
250 | * @param string $migration |
||
251 | * |
||
252 | * @return bool |
||
253 | */ |
||
254 | public function deleteMigrationFile($migration) |
||
258 | |||
259 | /** |
||
260 | * Get array of migrations that should be ran. |
||
261 | * |
||
262 | * @return array |
||
263 | */ |
||
264 | public function getMigrationsToRun() |
||
272 | |||
273 | /** |
||
274 | * Move migration files. |
||
275 | * |
||
276 | * @param array $files |
||
277 | * @param string $toDir |
||
278 | * |
||
279 | * @return int |
||
280 | */ |
||
281 | public function moveMigrationFiles($files = [], $toDir = '') |
||
305 | |||
306 | /** |
||
307 | * Construct migration file name from migration name and current time. |
||
308 | * |
||
309 | * @param string $name |
||
310 | * |
||
311 | * @return string |
||
312 | */ |
||
313 | protected function constructFileName($name) |
||
321 | |||
322 | /** |
||
323 | * Get a migration class name by a migration file name. |
||
324 | * |
||
325 | * @param string $file |
||
326 | * |
||
327 | * @return string |
||
328 | */ |
||
329 | protected function getMigrationClassNameByFileName($file) |
||
338 | |||
339 | /** |
||
340 | * Replace all placeholders in the stub. |
||
341 | * |
||
342 | * @param string $template |
||
343 | * @param array $replace |
||
344 | * |
||
345 | * @return string |
||
346 | */ |
||
347 | protected function replacePlaceholdersInTemplate($template, array $replace) |
||
355 | |||
356 | /** |
||
357 | * Resolve a migration instance from a file. |
||
358 | * |
||
359 | * @param string $file |
||
360 | * |
||
361 | * @throws Exception |
||
362 | * |
||
363 | * @return MigrationInterface |
||
364 | */ |
||
365 | protected function getMigrationObjectByFileName($file) |
||
379 | |||
380 | /** |
||
381 | * Require migration file. |
||
382 | * |
||
383 | * @param string $file |
||
384 | * |
||
385 | * @return void |
||
386 | */ |
||
387 | protected function requireMigrationFile($file) |
||
391 | |||
392 | /** |
||
393 | * Get path to a migration file. |
||
394 | * |
||
395 | * @param string $migration |
||
396 | * |
||
397 | * @return string |
||
398 | */ |
||
399 | protected function getMigrationFilePath($migration) |
||
408 | |||
409 | /** |
||
410 | * If package arrilot/bitrix-iblock-helper is loaded then we should disable its caching to avoid problems. |
||
411 | */ |
||
412 | private function disableBitrixIblockHelperCache() |
||
428 | |||
429 | /** |
||
430 | * @param MigrationInterface $migration |
||
431 | * @param callable $callback |
||
432 | * @throws Exception |
||
433 | */ |
||
434 | protected function checkTransactionAndRun($migration, $callback) |
||
452 | } |
||
453 |
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.