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 MSSQLSchemaManager 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 MSSQLSchemaManager, and based on these observations, apply Extract Interface, too.
| 1 | <?php | ||
| 11 | class MSSQLSchemaManager extends DBSchemaManager | ||
| 12 | { | ||
| 13 | |||
| 14 | /** | ||
| 15 | * Stores per-request cached constraint checks that come from the database. | ||
| 16 | * | ||
| 17 | * @var array | ||
| 18 | */ | ||
| 19 | protected static $cached_checks = []; | ||
| 20 | |||
| 21 | /** | ||
| 22 | * Builds the internal MS SQL Server index name given the silverstripe table and index name | ||
| 23 | * | ||
| 24 | * @param string $tableName | ||
| 25 | * @param string $indexName | ||
| 26 | * @param string $prefix The optional prefix for the index. Defaults to "ix" for indexes. | ||
| 27 | * @return string The name of the index | ||
| 28 | */ | ||
| 29 | public function buildMSSQLIndexName($tableName, $indexName, $prefix = 'ix') | ||
| 36 | |||
| 37 | |||
| 38 | /** | ||
| 39 | * This will set up the full text search capabilities. | ||
| 40 | * | ||
| 41 | * @param string $name Name of full text catalog to use | ||
| 42 | */ | ||
| 43 | public function createFullTextCatalog($name = 'ftCatalog') | ||
| 50 | |||
| 51 | /** | ||
| 52 | * Check that a fulltext catalog has been created yet. | ||
| 53 | * | ||
| 54 | * @param string $name Name of full text catalog to use | ||
| 55 | * @return boolean | ||
| 56 | */ | ||
| 57 | public function fullTextCatalogExists($name = 'ftCatalog') | ||
| 64 | |||
| 65 | /** | ||
| 66 | * Sleep until the catalog has been fully rebuilt. This is a busy wait designed for situations | ||
| 67 | * when you need to be sure the index is up to date - for example in unit tests. | ||
| 68 | * | ||
| 69 | * TODO: move this to Database class? Can we assume this will be useful for all databases? | ||
| 70 | * Also see the wrapper functions "waitUntilIndexingFinished" in SearchFormTest and TranslatableSearchFormTest | ||
| 71 | * | ||
| 72 | * @param int $maxWaitingTime Time in seconds to wait for the database. | ||
| 73 | */ | ||
| 74 | public function waitUntilIndexingFinished($maxWaitingTime = 15) | ||
| 94 | |||
| 95 | /** | ||
| 96 | * Check if a fulltext index exists on a particular table name. | ||
| 97 | * | ||
| 98 | * @param string $tableName | ||
| 99 | * @return boolean TRUE index exists | FALSE index does not exist | NULL no support | ||
| 100 | */ | ||
| 101 | public function fulltextIndexExists($tableName) | ||
| 115 | |||
| 116 | /** | ||
| 117 | * MSSQL stores the primary key column with an internal identifier, | ||
| 118 | * so a lookup needs to be done to determine it. | ||
| 119 | * | ||
| 120 | * @param string $tableName Name of table with primary key column "ID" | ||
| 121 | * @return string Internal identifier for primary key | ||
| 122 | */ | ||
| 123 | public function getPrimaryKey($tableName) | ||
| 136 | |||
| 137 | public function createDatabase($name) | ||
| 141 | |||
| 142 | public function dropDatabase($name) | ||
| 146 | |||
| 147 | public function databaseExists($name) | ||
| 158 | |||
| 159 | public function databaseList() | ||
| 163 | |||
| 164 | /** | ||
| 165 | * Create a new table. | ||
| 166 | * | ||
| 167 | * @param string $tableName The name of the table | ||
| 168 | * @param array $fields A map of field names to field types | ||
| 169 | * @param array $indexes A map of indexes | ||
| 170 | * @param array $options An map of additional options. The available keys are as follows: | ||
| 171 | * - 'MSSQLDatabase'/'MySQLDatabase'/'PostgreSQLDatabase' - database-specific options such as "engine" for MySQL. | ||
| 172 | * - 'temporary' - If true, then a temporary table will be created | ||
| 173 | * @param array $advancedOptions | ||
| 174 | * @return string The table name generated. This may be different from the table name, for example with temporary tables. | ||
| 175 | */ | ||
| 176 | public function createTable($tableName, $fields = null, $indexes = null, $options = null, $advancedOptions = null) | ||
| 213 | |||
| 214 | /** | ||
| 215 | * Alter a table's schema. | ||
| 216 | * @param string $tableName The name of the table to alter | ||
| 217 | * @param array $newFields New fields, a map of field name => field schema | ||
| 218 | * @param array $newIndexes New indexes, a map of index name => index type | ||
| 219 | * @param array $alteredFields Updated fields, a map of field name => field schema | ||
| 220 | * @param array $alteredIndexes Updated indexes, a map of index name => index type | ||
| 221 | * @param array $alteredOptions | ||
| 222 | * @param array $advancedOptions | ||
| 223 | */ | ||
| 224 | public function alterTable($tableName, $newFields = null, $newIndexes = null, $alteredFields = null, $alteredIndexes = null, $alteredOptions=null, $advancedOptions=null) | ||
| 265 | |||
| 266 | /** | ||
| 267 | * Given the table and column name, retrieve the constraint name for that column | ||
| 268 | * in the table. | ||
| 269 | * | ||
| 270 | * @param string $tableName Table name column resides in | ||
| 271 | * @param string $columnName Column name the constraint is for | ||
| 272 | * @return string|null | ||
| 273 | */ | ||
| 274 | public function getConstraintName($tableName, $columnName) | ||
| 283 | |||
| 284 | /** | ||
| 285 | * Given a table and column name, return a check constraint clause for that column in | ||
| 286 | * the table. | ||
| 287 | * | ||
| 288 | * This is an expensive query, so it is cached per-request and stored by table. The initial | ||
| 289 | * call for a table that has not been cached will query all columns and store that | ||
| 290 | * so subsequent calls are fast. | ||
| 291 | * | ||
| 292 | * @param string $tableName Table name column resides in | ||
| 293 | * @param string $columnName Column name the constraint is for | ||
| 294 | * @return string The check string | ||
| 295 | */ | ||
| 296 | public function getConstraintCheckClause($tableName, $columnName) | ||
| 322 | |||
| 323 | /** | ||
| 324 | * Return the name of the default constraint applied to $tableName.$colName. | ||
| 325 | * Will return null if no such constraint exists | ||
| 326 | * | ||
| 327 | * @param string $tableName Name of the table | ||
| 328 | * @param string $colName Name of the column | ||
| 329 | * @return string|null | ||
| 330 | */ | ||
| 331 | protected function defaultConstraintName($tableName, $colName) | ||
| 344 | |||
| 345 | /** | ||
| 346 | * Get enum values from a constraint check clause. | ||
| 347 | * | ||
| 348 | * @param string $clause Check clause to parse values from | ||
| 349 | * @return array Enum values | ||
| 350 | */ | ||
| 351 | protected function enumValuesFromCheckClause($clause) | ||
| 363 | |||
| 364 | /* | ||
| 365 | * Creates an ALTER expression for a column in MS SQL | ||
| 366 | * | ||
| 367 | * @param string $tableName Name of the table to be altered | ||
| 368 | * @param string $colName Name of the column to be altered | ||
| 369 | * @param string $colSpec String which contains conditions for a column | ||
| 370 | * @return string | ||
| 371 | */ | ||
| 372 | protected function alterTableAlterColumn($tableName, $colName, $colSpec) | ||
| 430 | |||
| 431 | /** | ||
| 432 | * @param string $oldTableName | ||
| 433 | * @param string $newTableName | ||
| 434 | */ | ||
| 435 | public function renameTable($oldTableName, $newTableName) | ||
| 439 | |||
| 440 | /** | ||
| 441 | * Checks a table's integrity and repairs it if necessary. | ||
| 442 | * NOTE: MSSQL does not appear to support any vacuum or optimise commands | ||
| 443 | * | ||
| 444 | * @var string $tableName The name of the table. | ||
| 445 | * @return boolean Return true if the table has integrity after the method is complete. | ||
| 446 | */ | ||
| 447 | public function checkAndRepairTable($tableName) | ||
| 451 | |||
| 452 | /** | ||
| 453 | * @param string $tableName | ||
| 454 | * @param string $fieldName | ||
| 455 | * @param string $fieldSpec | ||
| 456 | */ | ||
| 457 | public function createField($tableName, $fieldName, $fieldSpec) | ||
| 461 | |||
| 462 | /** | ||
| 463 | * Change the database type of the given field. | ||
| 464 | * | ||
| 465 | * @param string $tableName The name of the tbale the field is in. | ||
| 466 | * @param string $fieldName The name of the field to change. | ||
| 467 | * @param string $fieldSpec The new field specification | ||
| 468 | */ | ||
| 469 | public function alterField($tableName, $fieldName, $fieldSpec) | ||
| 473 | |||
| 474 | /** | ||
| 475 | * @param string $tableName | ||
| 476 | * @param string $oldName | ||
| 477 | * @param string $newName | ||
| 478 | * | ||
| 479 | */ | ||
| 480 | public function renameField($tableName, $oldName, $newName) | ||
| 484 | |||
| 485 | /** | ||
| 486 | * @param string $table | ||
| 487 | * | ||
| 488 | * @return array | ||
| 489 | */ | ||
| 490 | public function fieldList($table) | ||
| 600 | |||
| 601 | /** | ||
| 602 | * Create an index on a table. | ||
| 603 | * @param string $tableName The name of the table. | ||
| 604 | * @param string $indexName The name of the index. | ||
| 605 | * @param string $indexSpec The specification of the index, see SS_Database::requireIndex() for more details. | ||
| 606 | */ | ||
| 607 | public function createIndex($tableName, $indexName, $indexSpec) | ||
| 611 | |||
| 612 | |||
| 613 | /** | ||
| 614 | * This takes the index spec which has been provided by a class (ie static $indexes = blah blah) | ||
| 615 | * and turns it into a proper string. | ||
| 616 | * Some indexes may be arrays, such as fulltext and unique indexes, and this allows database-specific | ||
| 617 |      * arrays to be created. See {@link requireTable()} for details on the index format. | ||
| 618 | * | ||
| 619 | * @see http://dev.mysql.com/doc/refman/5.0/en/create-index.html | ||
| 620 | * @see parseIndexSpec() for approximate inverse | ||
| 621 | * | ||
| 622 | * @param string|array $indexSpec | ||
| 623 | * @return string | ||
| 624 | */ | ||
| 625 | protected function convertIndexSpec($indexSpec) | ||
| 632 | |||
| 633 | /** | ||
| 634 | * Return SQL for dropping and recreating an index | ||
| 635 | * | ||
| 636 | * @param string $tableName Name of table to create this index against | ||
| 637 | * @param string $indexName Name of this index | ||
| 638 | * @param array|string $indexSpec Index specification, either as a raw string | ||
| 639 | * or parsed array form | ||
| 640 | * @return string The SQL required to generate this index | ||
| 641 | */ | ||
| 642 | protected function getIndexSqlDefinition($tableName, $indexName, $indexSpec) | ||
| 673 | |||
| 674 | public function alterIndex($tableName, $indexName, $indexSpec) | ||
| 678 | |||
| 679 | /** | ||
| 680 | * Return the list of indexes in a table. | ||
| 681 | * | ||
| 682 | * @param string $table The table name. | ||
| 683 | * @return array | ||
| 684 | */ | ||
| 685 | public function indexList($table) | ||
| 732 | |||
| 733 | /** | ||
| 734 | * For a given table name, get all the internal index names, | ||
| 735 | * except for those that are primary keys and fulltext indexes. | ||
| 736 | * | ||
| 737 | * @param string $tableName | ||
| 738 | * @return array | ||
| 739 | */ | ||
| 740 | public function indexNames($tableName) | ||
| 749 | |||
| 750 | public function tableList() | ||
| 760 | |||
| 761 | /** | ||
| 762 | * Return a boolean type-formatted string | ||
| 763 | * We use 'bit' so that we can do numeric-based comparisons | ||
| 764 | * | ||
| 765 | * @param array $values Contains a tokenised list of info about this data type | ||
| 766 | * @return string | ||
| 767 | */ | ||
| 768 | public function boolean($values) | ||
| 773 | |||
| 774 | /** | ||
| 775 | * Return a date type-formatted string. | ||
| 776 | * | ||
| 777 | * @param array $values Contains a tokenised list of info about this data type | ||
| 778 | * @return string | ||
| 779 | */ | ||
| 780 | public function date($values) | ||
| 784 | |||
| 785 | /** | ||
| 786 | * Return a decimal type-formatted string | ||
| 787 | * | ||
| 788 | * @param array $values Contains a tokenised list of info about this data type | ||
| 789 | * @return string | ||
| 790 | */ | ||
| 791 | public function decimal($values) | ||
| 808 | |||
| 809 | /** | ||
| 810 | * Return a enum type-formatted string | ||
| 811 | * | ||
| 812 | * @param array $values Contains a tokenised list of info about this data type | ||
| 813 | * @return string | ||
| 814 | */ | ||
| 815 | public function enum($values) | ||
| 827 | |||
| 828 | /** | ||
| 829 |      * @todo Make this work like {@link MySQLDatabase::set()} | ||
| 830 | * | ||
| 831 | * @param array $values | ||
| 832 | * @return string | ||
| 833 | */ | ||
| 834 | public function set($values) | ||
| 838 | |||
| 839 | /** | ||
| 840 | * Return a float type-formatted string. | ||
| 841 | * | ||
| 842 | * @param array $values Contains a tokenised list of info about this data type | ||
| 843 | * @return string | ||
| 844 | */ | ||
| 845 | public function float($values) | ||
| 849 | |||
| 850 | /** | ||
| 851 | * Return a int type-formatted string | ||
| 852 | * | ||
| 853 | * @param array $values Contains a tokenised list of info about this data type | ||
| 854 | * @return string | ||
| 855 | */ | ||
| 856 | public function int($values) | ||
| 860 | |||
| 861 | /** | ||
| 862 | * Return a bigint type-formatted string | ||
| 863 | * | ||
| 864 | * @param array $values Contains a tokenised list of info about this data type | ||
| 865 | * @return string | ||
| 866 | */ | ||
| 867 | public function bigint($values) | ||
| 871 | |||
| 872 | /** | ||
| 873 | * Return a datetime type-formatted string | ||
| 874 | * For MS SQL, we simply return the word 'timestamp', no other parameters are necessary | ||
| 875 | * | ||
| 876 | * @param array $values Contains a tokenised list of info about this data type | ||
| 877 | * @return string | ||
| 878 | */ | ||
| 879 | public function datetime($values) | ||
| 883 | |||
| 884 | /** | ||
| 885 | * Return a text type-formatted string | ||
| 886 | * | ||
| 887 | * @param array $values Contains a tokenised list of info about this data type | ||
| 888 | * @return string | ||
| 889 | */ | ||
| 890 | public function text($values) | ||
| 896 | |||
| 897 | /** | ||
| 898 | * Return a time type-formatted string. | ||
| 899 | * | ||
| 900 | * @param array $values Contains a tokenised list of info about this data type | ||
| 901 | * @return string | ||
| 902 | */ | ||
| 903 | public function time($values) | ||
| 907 | |||
| 908 | /** | ||
| 909 | * Return a varchar type-formatted string | ||
| 910 | * | ||
| 911 | * @param array $values Contains a tokenised list of info about this data type | ||
| 912 | * @return string | ||
| 913 | */ | ||
| 914 | public function varchar($values) | ||
| 920 | |||
| 921 | /** | ||
| 922 | * Return a 4 digit numeric type. | ||
| 923 | * | ||
| 924 | * @param array $values | ||
| 925 | * @return string | ||
| 926 | */ | ||
| 927 | public function year($values) | ||
| 931 | |||
| 932 | /** | ||
| 933 | * This returns the column which is the primary key for each table | ||
| 934 | * | ||
| 935 | * @param bool $asDbValue | ||
| 936 | * @param bool $hasAutoIncPK | ||
| 937 | * @return string | ||
| 938 | */ | ||
| 939 | public function IdColumn($asDbValue = false, $hasAutoIncPK = true) | ||
| 949 | |||
| 950 | public function hasTable($tableName) | ||
| 957 | |||
| 958 | /** | ||
| 959 | * Returns the values of the given enum field | ||
| 960 | * NOTE: Experimental; introduced for db-abstraction and may changed before 2.4 is released. | ||
| 961 | * | ||
| 962 | * @param string $tableName | ||
| 963 | * @param string $fieldName | ||
| 964 | * @return array | ||
| 965 | */ | ||
| 966 | public function enumValuesForField($tableName, $fieldName) | ||
| 978 | |||
| 979 | /** | ||
| 980 | * @param string $type | ||
| 981 | * @return string | ||
| 982 | */ | ||
| 983 | public function dbDataType($type) | ||
| 995 | |||
| 996 | /** | ||
| 997 | * @param string $tableName | ||
| 998 | * @param string $indexName | ||
| 999 | * @param mixed $spec | ||
| 1000 | */ | ||
| 1001 | protected function indexKey($tableName, $indexName, $spec) | ||
| 1005 | } | ||
| 1006 | 
Let’s take a look at an example:
In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.
Available Fixes
Change the type-hint for the parameter:
Add an additional type-check:
Add the method to the parent class: