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 MigrationService 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 MigrationService, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
24 | class MigrationService implements ContextProviderInterface |
||
25 | { |
||
26 | use RepositoryUserSetterTrait; |
||
27 | |||
28 | /** |
||
29 | * The default Admin user Id, used when no Admin user is specified |
||
30 | */ |
||
31 | const ADMIN_USER_ID = 14; |
||
32 | |||
33 | /** |
||
34 | * @var LoaderInterface $loader |
||
35 | */ |
||
36 | protected $loader; |
||
37 | /** |
||
38 | * @var StorageHandlerInterface $storageHandler |
||
39 | */ |
||
40 | protected $storageHandler; |
||
41 | |||
42 | /** @var DefinitionParserInterface[] $DefinitionParsers */ |
||
43 | protected $DefinitionParsers = array(); |
||
44 | |||
45 | /** @var ExecutorInterface[] $executors */ |
||
46 | protected $executors = array(); |
||
47 | |||
48 | protected $repository; |
||
49 | |||
50 | protected $dispatcher; |
||
51 | |||
52 | /** |
||
53 | * @var ContextHandler $contextHandler |
||
54 | */ |
||
55 | protected $contextHandler; |
||
56 | |||
57 | protected $eventPrefix = 'ez_migration.'; |
||
58 | |||
59 | protected $eventEntity = 'migration'; |
||
60 | |||
61 | protected $migrationContext = array(); |
||
62 | |||
63 | 70 | public function __construct(LoaderInterface $loader, StorageHandlerInterface $storageHandler, Repository $repository, |
|
64 | EventDispatcherInterface $eventDispatcher, $contextHandler) |
||
65 | { |
||
66 | 70 | $this->loader = $loader; |
|
67 | 70 | $this->storageHandler = $storageHandler; |
|
68 | 70 | $this->repository = $repository; |
|
69 | 70 | $this->dispatcher = $eventDispatcher; |
|
70 | 70 | $this->contextHandler = $contextHandler; |
|
71 | 70 | } |
|
72 | |||
73 | 70 | public function addDefinitionParser(DefinitionParserInterface $DefinitionParser) |
|
74 | { |
||
75 | 70 | $this->DefinitionParsers[] = $DefinitionParser; |
|
76 | 70 | } |
|
77 | |||
78 | 70 | public function addExecutor(ExecutorInterface $executor) |
|
79 | { |
||
80 | 70 | foreach ($executor->supportedTypes() as $type) { |
|
81 | 70 | $this->executors[$type] = $executor; |
|
82 | } |
||
83 | 70 | } |
|
84 | |||
85 | /** |
||
86 | * @param string $type |
||
87 | * @return ExecutorInterface |
||
88 | * @throws \InvalidArgumentException If executor doesn't exist |
||
89 | */ |
||
90 | 25 | public function getExecutor($type) |
|
91 | { |
||
92 | 25 | if (!isset($this->executors[$type])) { |
|
93 | throw new \InvalidArgumentException("Executor with type '$type' doesn't exist"); |
||
94 | } |
||
95 | |||
96 | 25 | return $this->executors[$type]; |
|
97 | } |
||
98 | |||
99 | /** |
||
100 | * @return string[] |
||
101 | */ |
||
102 | 25 | public function listExecutors() |
|
103 | { |
||
104 | 25 | return array_keys($this->executors); |
|
105 | } |
||
106 | |||
107 | /** |
||
108 | * NB: returns UNPARSED definitions |
||
109 | * |
||
110 | * @param string[] $paths |
||
111 | * @return MigrationDefinitionCollection key: migration name, value: migration definition as binary string |
||
112 | */ |
||
113 | 68 | public function getMigrationsDefinitions(array $paths = array()) |
|
114 | { |
||
115 | // we try to be flexible in file types we support, and the same time avoid loading all files in a directory |
||
116 | 68 | $handledDefinitions = array(); |
|
117 | 68 | foreach ($this->loader->listAvailableDefinitions($paths) as $migrationName => $definitionPath) { |
|
118 | 68 | foreach ($this->DefinitionParsers as $definitionParser) { |
|
119 | 68 | if ($definitionParser->supports($migrationName)) { |
|
120 | 68 | $handledDefinitions[] = $definitionPath; |
|
121 | } |
||
122 | } |
||
123 | } |
||
124 | |||
125 | // we can not call loadDefinitions with an empty array using the Filesystem loader, or it will start looking in bundles... |
||
126 | 68 | if (empty($handledDefinitions) && !empty($paths)) { |
|
127 | return new MigrationDefinitionCollection(); |
||
128 | } |
||
129 | |||
130 | 68 | return $this->loader->loadDefinitions($handledDefinitions); |
|
131 | } |
||
132 | |||
133 | /** |
||
134 | * Returns the list of all the migrations which where executed or attempted so far |
||
135 | * |
||
136 | * @param int $limit 0 or below will be treated as 'no limit' |
||
137 | * @param int $offset |
||
138 | * @return \Kaliop\eZMigrationBundle\API\Collection\MigrationCollection |
||
139 | */ |
||
140 | 69 | public function getMigrations($limit = null, $offset = null) |
|
141 | { |
||
142 | 69 | return $this->storageHandler->loadMigrations($limit, $offset); |
|
143 | } |
||
144 | |||
145 | /** |
||
146 | * Returns the list of all the migrations in a given status which where executed or attempted so far |
||
147 | * |
||
148 | * @param int $status |
||
149 | * @param int $limit 0 or below will be treated as 'no limit' |
||
150 | * @param int $offset |
||
151 | * @return \Kaliop\eZMigrationBundle\API\Collection\MigrationCollection |
||
152 | */ |
||
153 | 1 | public function getMigrationsByStatus($status, $limit = null, $offset = null) |
|
154 | { |
||
155 | 1 | return $this->storageHandler->loadMigrationsByStatus($status, $limit, $offset); |
|
156 | } |
||
157 | |||
158 | /** |
||
159 | * @param string $migrationName |
||
160 | * @return Migration|null |
||
161 | */ |
||
162 | 39 | public function getMigration($migrationName) |
|
163 | { |
||
164 | 39 | return $this->storageHandler->loadMigration($migrationName); |
|
165 | } |
||
166 | |||
167 | /** |
||
168 | * @param MigrationDefinition $migrationDefinition |
||
169 | * @return Migration |
||
170 | */ |
||
171 | 38 | public function addMigration(MigrationDefinition $migrationDefinition) |
|
175 | |||
176 | /** |
||
177 | * @param Migration $migration |
||
178 | */ |
||
179 | 36 | public function deleteMigration(Migration $migration) |
|
180 | { |
||
181 | 36 | return $this->storageHandler->deleteMigration($migration); |
|
182 | } |
||
183 | |||
184 | /** |
||
185 | * @param MigrationDefinition $migrationDefinition |
||
186 | * @return Migration |
||
187 | */ |
||
188 | public function skipMigration(MigrationDefinition $migrationDefinition) |
||
192 | |||
193 | /** |
||
194 | * Not to be called by external users for normal use cases, you should use executeMigration() instead |
||
195 | * |
||
196 | * @param Migration $migration |
||
197 | */ |
||
198 | public function endMigration(Migration $migration) |
||
202 | |||
203 | /** |
||
204 | * Parses a migration definition, return a parsed definition. |
||
205 | * If there is a parsing error, the definition status will be updated accordingly |
||
206 | * |
||
207 | * @param MigrationDefinition $migrationDefinition |
||
208 | * @return MigrationDefinition |
||
209 | * @throws \Exception if the migrationDefinition has no suitable parser for its source format |
||
210 | */ |
||
211 | 70 | public function parseMigrationDefinition(MigrationDefinition $migrationDefinition) |
|
238 | |||
239 | /** |
||
240 | * @param MigrationDefinition $migrationDefinition |
||
241 | * @param bool $useTransaction when set to false, no repo transaction will be used to wrap the migration |
||
242 | * @param string $defaultLanguageCode |
||
243 | * @param string|int|false|null $adminLogin when false, current user is used; when null, hardcoded admin account |
||
244 | * @throws \Exception |
||
245 | * |
||
246 | * @todo treating a null and false $adminLogin values differently is prone to hard-to-track errors. |
||
247 | * Shall we use instead -1 to indicate the desire to not-login-as-admin-user-at-all ? |
||
248 | */ |
||
249 | 30 | public function executeMigration(MigrationDefinition $migrationDefinition, $useTransaction = true, |
|
268 | |||
269 | /** |
||
270 | * @param Migration $migration |
||
271 | * @param MigrationDefinition $migrationDefinition |
||
272 | * @param array $migrationContext |
||
273 | * @param int $stepOffset |
||
274 | * @param bool $useTransaction when set to false, no repo transaction will be used to wrap the migration |
||
275 | * @param string|int|false|null $adminLogin used only for committing db transaction if needed. If false or null, hardcoded admin is used |
||
276 | * @throws \Exception |
||
277 | */ |
||
278 | 30 | protected function executeMigrationInner(Migration $migration, MigrationDefinition $migrationDefinition, |
|
405 | |||
406 | /** |
||
407 | * @param Migration $migration |
||
408 | * @param bool $useTransaction |
||
409 | * @throws \Exception |
||
410 | * |
||
411 | * @todo add support for adminLogin ? |
||
412 | */ |
||
413 | public function resumeMigration(Migration $migration, $useTransaction = true) |
||
454 | |||
455 | /** |
||
456 | * @param string $defaultLanguageCode |
||
457 | * @param string|int|false $adminLogin |
||
458 | * @return array |
||
459 | */ |
||
460 | 30 | protected function migrationContextFromParameters($defaultLanguageCode = null, $adminLogin = null) |
|
474 | |||
475 | 30 | protected function injectContextIntoStep(MigrationStep $step, array $context) |
|
483 | |||
484 | /** |
||
485 | * @param string $adminLogin |
||
486 | * @return int|string |
||
487 | */ |
||
488 | 2 | protected function getAdminUserIdentifier($adminLogin) |
|
496 | |||
497 | /** |
||
498 | * Turns eZPublish cryptic exceptions into something more palatable for random devs |
||
499 | * @todo should this be moved to a lower layer ? |
||
500 | * |
||
501 | * @param \Exception $e |
||
502 | * @return string |
||
503 | */ |
||
504 | 4 | protected function getFullExceptionMessage(\Exception $e) |
|
551 | |||
552 | /** |
||
553 | * @param string $migrationName |
||
554 | * @return array |
||
555 | */ |
||
556 | 1 | public function getCurrentContext($migrationName) |
|
560 | |||
561 | /** |
||
562 | * @param string $migrationName |
||
563 | * @param array $context |
||
564 | */ |
||
565 | public function restoreContext($migrationName, array $context) |
||
569 | |||
570 | protected function getEntityName($migration) |
||
574 | } |
||
575 |
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.