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: