cakephp /
phinx
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 TablePrefixAdapter 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 TablePrefixAdapter, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 55 | class TablePrefixAdapter extends AdapterWrapper implements DirectActionInterface |
||
| 56 | 2 | { |
|
| 57 | /** |
||
| 58 | 2 | * {@inheritdoc} |
|
| 59 | 2 | */ |
|
| 60 | public function getAdapterType() |
||
| 64 | |||
| 65 | 3 | /** |
|
| 66 | * {@inheritdoc} |
||
| 67 | 3 | */ |
|
| 68 | 3 | public function hasTable($tableName) |
|
| 74 | 1 | ||
| 75 | 3 | /** |
|
| 76 | * {@inheritdoc} |
||
| 77 | 3 | */ |
|
| 78 | 3 | public function createTable(Table $table, array $columns = [], array $indexes = []) |
|
| 79 | { |
||
| 80 | $adapterTable = new Table( |
||
| 81 | $this->getAdapterTableName($table->getName()), |
||
| 82 | $table->getOptions() |
||
| 83 | 1 | ); |
|
| 84 | parent::createTable($adapterTable, $columns, $indexes); |
||
| 85 | 1 | } |
|
| 86 | 1 | ||
| 87 | 1 | /** |
|
| 88 | 1 | * {@inheritdoc} |
|
| 89 | */ |
||
| 90 | public function changePrimaryKey(Table $table, $newColumns) |
||
| 98 | |||
| 99 | /** |
||
| 100 | * {@inheritdoc} |
||
| 101 | */ |
||
| 102 | public function changeComment(Table $table, $newComment) |
||
| 103 | { |
||
| 104 | $adapterTable = new Table( |
||
| 105 | $this->getAdapterTableName($table->getName()), |
||
| 106 | $table->getOptions() |
||
| 107 | ); |
||
| 108 | parent::changeComment($adapterTable, $newComment); |
||
| 109 | } |
||
| 110 | |||
| 111 | 1 | /** |
|
| 112 | * {@inheritdoc} |
||
| 113 | 1 | */ |
|
| 114 | 1 | public function renameTable($tableName, $newTableName) |
|
| 125 | |||
| 126 | /** |
||
| 127 | * {@inheritdoc} |
||
| 128 | */ |
||
| 129 | 1 | View Code Duplication | public function dropTable($tableName) |
| 138 | |||
| 139 | /** |
||
| 140 | 1 | * {@inheritdoc} |
|
| 141 | */ |
||
| 142 | 1 | public function truncateTable($tableName) |
|
| 143 | 1 | { |
|
| 144 | 1 | $adapterTableName = $this->getAdapterTableName($tableName); |
|
| 145 | parent::truncateTable($adapterTableName); |
||
| 146 | } |
||
| 147 | |||
| 148 | /** |
||
| 149 | 1 | * {@inheritdoc} |
|
| 150 | */ |
||
| 151 | 1 | public function getColumns($tableName) |
|
| 157 | |||
| 158 | 1 | /** |
|
| 159 | * {@inheritdoc} |
||
| 160 | 1 | */ |
|
| 161 | 1 | public function hasColumn($tableName, $columnName) |
|
| 167 | 1 | ||
| 168 | /** |
||
| 169 | 1 | * {@inheritdoc} |
|
| 170 | 1 | */ |
|
| 171 | View Code Duplication | public function addColumn(Table $table, Column $column) |
|
| 181 | |||
| 182 | /** |
||
| 183 | * {@inheritdoc} |
||
| 184 | */ |
||
| 185 | View Code Duplication | public function renameColumn($tableName, $columnName, $newColumnName) |
|
| 194 | |||
| 195 | /** |
||
| 196 | 1 | * {@inheritdoc} |
|
| 197 | */ |
||
| 198 | 1 | View Code Duplication | public function changeColumn($tableName, $columnName, Column $newColumn) |
| 207 | 1 | ||
| 208 | 1 | /** |
|
| 209 | 1 | * {@inheritdoc} |
|
| 210 | */ |
||
| 211 | View Code Duplication | public function dropColumn($tableName, $columnName) |
|
| 220 | |||
| 221 | /** |
||
| 222 | * {@inheritdoc} |
||
| 223 | 1 | */ |
|
| 224 | public function hasIndex($tableName, $columns) |
||
| 230 | |||
| 231 | /** |
||
| 232 | * {@inheritdoc} |
||
| 233 | */ |
||
| 234 | 1 | public function hasIndexByName($tableName, $indexName) |
|
| 240 | |||
| 241 | /** |
||
| 242 | * {@inheritdoc} |
||
| 243 | */ |
||
| 244 | public function addIndex(Table $table, Index $index) |
||
| 253 | |||
| 254 | 1 | /** |
|
| 255 | * {@inheritdoc} |
||
| 256 | 1 | */ |
|
| 257 | 1 | View Code Duplication | public function dropIndex($tableName, $columns) |
| 266 | |||
| 267 | 19 | /** |
|
| 268 | * {@inheritdoc} |
||
| 269 | 19 | */ |
|
| 270 | View Code Duplication | public function dropIndexByName($tableName, $indexName) |
|
| 279 | 19 | ||
| 280 | /** |
||
| 281 | * {@inheritdoc} |
||
| 282 | */ |
||
| 283 | public function hasPrimaryKey($tableName, $columns, $constraint = null) |
||
| 289 | |||
| 290 | 19 | /** |
|
| 291 | * {@inheritdoc} |
||
| 292 | */ |
||
| 293 | public function hasForeignKey($tableName, $columns, $constraint = null) |
||
| 299 | |||
| 300 | /** |
||
| 301 | * {@inheritdoc} |
||
| 302 | */ |
||
| 303 | View Code Duplication | public function addForeignKey(Table $table, ForeignKey $foreignKey) |
|
| 313 | |||
| 314 | /** |
||
| 315 | * {@inheritdoc} |
||
| 316 | */ |
||
| 317 | View Code Duplication | public function dropForeignKey($tableName, $columns, $constraint = null) |
|
| 326 | |||
| 327 | /** |
||
| 328 | * {@inheritdoc} |
||
| 329 | */ |
||
| 330 | public function insert(Table $table, $row) |
||
| 331 | { |
||
| 332 | $adapterTableName = $this->getAdapterTableName($table->getName()); |
||
| 333 | $adapterTable = new Table($adapterTableName, $table->getOptions()); |
||
| 334 | parent::insert($adapterTable, $row); |
||
| 335 | } |
||
| 336 | |||
| 337 | /** |
||
| 338 | * {@inheritdoc} |
||
| 339 | */ |
||
| 340 | public function bulkinsert(Table $table, $rows) |
||
| 346 | |||
| 347 | /** |
||
| 348 | * Gets the table prefix. |
||
| 349 | * |
||
| 350 | * @return string |
||
| 351 | */ |
||
| 352 | public function getPrefix() |
||
| 356 | |||
| 357 | /** |
||
| 358 | * Gets the table suffix. |
||
| 359 | * |
||
| 360 | * @return string |
||
| 361 | */ |
||
| 362 | public function getSuffix() |
||
| 366 | |||
| 367 | /** |
||
| 368 | * Applies the prefix and suffix to the table name. |
||
| 369 | * |
||
| 370 | * @param string $tableName |
||
| 371 | * @return string |
||
| 372 | */ |
||
| 373 | public function getAdapterTableName($tableName) |
||
| 377 | |||
| 378 | /** |
||
| 379 | * {@inheritdoc} |
||
| 380 | */ |
||
| 381 | public function executeActions(Table $table, array $actions) |
||
| 449 | } |
||
| 450 |
Loading history...
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: