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 |