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 Comparator 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 Comparator, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
31 | class Comparator |
||
32 | { |
||
33 | /** |
||
34 | * @param \Doctrine\DBAL\Schema\Schema $fromSchema |
||
35 | * @param \Doctrine\DBAL\Schema\Schema $toSchema |
||
36 | * |
||
37 | * @return \Doctrine\DBAL\Schema\SchemaDiff |
||
38 | */ |
||
39 | 17 | static public function compareSchemas(Schema $fromSchema, Schema $toSchema) |
|
45 | |||
46 | /** |
||
47 | * Returns a SchemaDiff object containing the differences between the schemas $fromSchema and $toSchema. |
||
48 | * |
||
49 | * The returned differences are returned in such a way that they contain the |
||
50 | * operations to change the schema stored in $fromSchema to the schema that is |
||
51 | * stored in $toSchema. |
||
52 | * |
||
53 | * @param \Doctrine\DBAL\Schema\Schema $fromSchema |
||
54 | * @param \Doctrine\DBAL\Schema\Schema $toSchema |
||
55 | * |
||
56 | * @return \Doctrine\DBAL\Schema\SchemaDiff |
||
57 | */ |
||
58 | 27 | public function compare(Schema $fromSchema, Schema $toSchema) |
|
157 | |||
158 | /** |
||
159 | * @param \Doctrine\DBAL\Schema\Schema $schema |
||
160 | * @param \Doctrine\DBAL\Schema\Sequence $sequence |
||
161 | * |
||
162 | * @return boolean |
||
163 | */ |
||
164 | 6 | private function isAutoIncrementSequenceInSchema($schema, $sequence) |
|
174 | |||
175 | /** |
||
176 | * @param \Doctrine\DBAL\Schema\Sequence $sequence1 |
||
177 | * @param \Doctrine\DBAL\Schema\Sequence $sequence2 |
||
178 | * |
||
179 | * @return boolean |
||
180 | */ |
||
181 | 3 | public function diffSequence(Sequence $sequence1, Sequence $sequence2) |
|
193 | |||
194 | /** |
||
195 | * Returns the difference between the tables $table1 and $table2. |
||
196 | * |
||
197 | * If there are no differences this method returns the boolean false. |
||
198 | * |
||
199 | * @param \Doctrine\DBAL\Schema\Table $table1 |
||
200 | * @param \Doctrine\DBAL\Schema\Table $table2 |
||
201 | * |
||
202 | * @return boolean|\Doctrine\DBAL\Schema\TableDiff |
||
203 | */ |
||
204 | 110 | public function diffTable(Table $table1, Table $table2) |
|
307 | |||
308 | /** |
||
309 | * Try to find columns that only changed their name, rename operations maybe cheaper than add/drop |
||
310 | * however ambiguities between different possibilities should not lead to renaming at all. |
||
311 | * |
||
312 | * @param \Doctrine\DBAL\Schema\TableDiff $tableDifferences |
||
313 | * |
||
314 | * @return void |
||
315 | */ |
||
316 | 110 | View Code Duplication | private function detectColumnRenamings(TableDiff $tableDifferences) |
341 | |||
342 | /** |
||
343 | * Try to find indexes that only changed their name, rename operations maybe cheaper than add/drop |
||
344 | * however ambiguities between different possibilities should not lead to renaming at all. |
||
345 | * |
||
346 | * @param \Doctrine\DBAL\Schema\TableDiff $tableDifferences |
||
347 | * |
||
348 | * @return void |
||
349 | */ |
||
350 | 110 | View Code Duplication | private function detectIndexRenamings(TableDiff $tableDifferences) |
382 | |||
383 | /** |
||
384 | * @param \Doctrine\DBAL\Schema\ForeignKeyConstraint $key1 |
||
385 | * @param \Doctrine\DBAL\Schema\ForeignKeyConstraint $key2 |
||
386 | * |
||
387 | * @return boolean |
||
388 | */ |
||
389 | 7 | public function diffForeignKey(ForeignKeyConstraint $key1, ForeignKeyConstraint $key2) |
|
413 | |||
414 | /** |
||
415 | * Returns the difference between the fields $field1 and $field2. |
||
416 | * |
||
417 | * If there are differences this method returns $field2, otherwise the |
||
418 | * boolean false. |
||
419 | * |
||
420 | * @param \Doctrine\DBAL\Schema\Column $column1 |
||
421 | * @param \Doctrine\DBAL\Schema\Column $column2 |
||
422 | * |
||
423 | * @return array |
||
424 | */ |
||
425 | 127 | public function diffColumn(Column $column1, Column $column2) |
|
426 | { |
||
427 | 127 | $properties1 = $column1->toArray(); |
|
428 | 127 | $properties2 = $column2->toArray(); |
|
429 | |||
430 | 127 | $changedProperties = array(); |
|
431 | |||
432 | 127 | foreach (array('type', 'notnull', 'unsigned', 'autoincrement') as $property) { |
|
433 | 127 | if ($properties1[$property] != $properties2[$property]) { |
|
434 | 127 | $changedProperties[] = $property; |
|
435 | } |
||
436 | } |
||
437 | |||
438 | 127 | if ($properties1['default'] != $properties2['default'] || |
|
439 | // Null values need to be checked additionally as they tell whether to create or drop a default value. |
||
440 | // null != 0, null != false, null != '' etc. This affects platform's table alteration SQL generation. |
||
441 | 124 | (null === $properties1['default'] && null !== $properties2['default']) || |
|
442 | 127 | (null === $properties2['default'] && null !== $properties1['default']) |
|
443 | ) { |
||
444 | 4 | $changedProperties[] = 'default'; |
|
445 | } |
||
446 | |||
447 | 127 | if (($properties1['type'] instanceof Types\StringType && ! $properties1['type'] instanceof Types\GuidType) || |
|
448 | 127 | $properties1['type'] instanceof Types\BinaryType |
|
449 | ) { |
||
450 | // check if value of length is set at all, default value assumed otherwise. |
||
451 | 25 | $length1 = $properties1['length'] ?: 255; |
|
452 | 25 | $length2 = $properties2['length'] ?: 255; |
|
453 | 25 | if ($length1 != $length2) { |
|
454 | 12 | $changedProperties[] = 'length'; |
|
455 | } |
||
456 | |||
457 | 25 | if ($properties1['fixed'] != $properties2['fixed']) { |
|
458 | 25 | $changedProperties[] = 'fixed'; |
|
459 | } |
||
460 | 112 | } elseif ($properties1['type'] instanceof Types\DecimalType) { |
|
461 | 2 | if (($properties1['precision'] ?: 10) != ($properties2['precision'] ?: 10)) { |
|
462 | $changedProperties[] = 'precision'; |
||
463 | } |
||
464 | 2 | if ($properties1['scale'] != $properties2['scale']) { |
|
465 | $changedProperties[] = 'scale'; |
||
466 | } |
||
467 | } |
||
468 | |||
469 | // A null value and an empty string are actually equal for a comment so they should not trigger a change. |
||
470 | 127 | if ($properties1['comment'] !== $properties2['comment'] && |
|
471 | 127 | ! (null === $properties1['comment'] && '' === $properties2['comment']) && |
|
472 | 127 | ! (null === $properties2['comment'] && '' === $properties1['comment']) |
|
473 | ) { |
||
474 | 44 | $changedProperties[] = 'comment'; |
|
475 | } |
||
476 | |||
477 | 127 | $customOptions1 = $column1->getCustomSchemaOptions(); |
|
478 | 127 | $customOptions2 = $column2->getCustomSchemaOptions(); |
|
479 | |||
480 | 127 | foreach (array_merge(array_keys($customOptions1), array_keys($customOptions2)) as $key) { |
|
481 | 2 | if ( ! array_key_exists($key, $properties1) || ! array_key_exists($key, $properties2)) { |
|
482 | 1 | $changedProperties[] = $key; |
|
483 | 2 | } elseif ($properties1[$key] !== $properties2[$key]) { |
|
484 | 2 | $changedProperties[] = $key; |
|
485 | } |
||
486 | } |
||
487 | |||
488 | 127 | $platformOptions1 = $column1->getPlatformOptions(); |
|
489 | 127 | $platformOptions2 = $column2->getPlatformOptions(); |
|
490 | |||
491 | 127 | foreach (array_keys(array_intersect_key($platformOptions1, $platformOptions2)) as $key) { |
|
492 | 2 | if ($properties1[$key] !== $properties2[$key]) { |
|
493 | 2 | $changedProperties[] = $key; |
|
494 | } |
||
495 | } |
||
496 | |||
497 | 127 | return array_unique($changedProperties); |
|
498 | } |
||
499 | |||
500 | /** |
||
501 | * Finds the difference between the indexes $index1 and $index2. |
||
502 | * |
||
503 | * Compares $index1 with $index2 and returns $index2 if there are any |
||
504 | * differences or false in case there are no differences. |
||
505 | * |
||
506 | * @param \Doctrine\DBAL\Schema\Index $index1 |
||
507 | * @param \Doctrine\DBAL\Schema\Index $index2 |
||
508 | * |
||
509 | * @return boolean |
||
510 | */ |
||
511 | 35 | public function diffIndex(Index $index1, Index $index2) |
|
519 | } |
||
520 |