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 AbstractSubject 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 AbstractSubject, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 50 | abstract class AbstractSubject implements SubjectInterface |
||
| 51 | { |
||
| 52 | |||
| 53 | /** |
||
| 54 | * The root directory for the virtual filesystem. |
||
| 55 | * |
||
| 56 | * @var string |
||
| 57 | */ |
||
| 58 | protected $rootDir; |
||
| 59 | |||
| 60 | /** |
||
| 61 | * The system configuration. |
||
| 62 | * |
||
| 63 | * @var \TechDivision\Import\Configuration\SubjectConfigurationInterface |
||
| 64 | */ |
||
| 65 | protected $configuration; |
||
| 66 | |||
| 67 | /** |
||
| 68 | * The system logger implementation. |
||
| 69 | * |
||
| 70 | * @var \Psr\Log\LoggerInterface |
||
| 71 | */ |
||
| 72 | protected $systemLogger; |
||
| 73 | |||
| 74 | /** |
||
| 75 | * The RegistryProcessor instance to handle running threads. |
||
| 76 | * |
||
| 77 | * @var \TechDivision\Import\Services\RegistryProcessorInterface |
||
| 78 | */ |
||
| 79 | protected $registryProcessor; |
||
| 80 | |||
| 81 | /** |
||
| 82 | * The actions unique serial. |
||
| 83 | * |
||
| 84 | * @var string |
||
| 85 | */ |
||
| 86 | protected $serial; |
||
| 87 | |||
| 88 | /** |
||
| 89 | * The name of the file to be imported. |
||
| 90 | * |
||
| 91 | * @var string |
||
| 92 | */ |
||
| 93 | protected $filename; |
||
| 94 | |||
| 95 | /** |
||
| 96 | * Array with the subject's observers. |
||
| 97 | * |
||
| 98 | * @var array |
||
| 99 | */ |
||
| 100 | protected $observers = array(); |
||
| 101 | |||
| 102 | /** |
||
| 103 | * Array with the subject's callbacks. |
||
| 104 | * |
||
| 105 | * @var array |
||
| 106 | */ |
||
| 107 | protected $callbacks = array(); |
||
| 108 | |||
| 109 | /** |
||
| 110 | * The subject's callback mappings. |
||
| 111 | * |
||
| 112 | * @var array |
||
| 113 | */ |
||
| 114 | protected $callbackMappings = array(); |
||
| 115 | |||
| 116 | /** |
||
| 117 | * Contain's the column names from the header line. |
||
| 118 | * |
||
| 119 | * @var array |
||
| 120 | */ |
||
| 121 | protected $headers = array(); |
||
| 122 | |||
| 123 | /** |
||
| 124 | * The virtual filesystem instance. |
||
| 125 | * |
||
| 126 | * @var \League\Flysystem\FilesystemInterface |
||
| 127 | */ |
||
| 128 | protected $filesystem; |
||
| 129 | |||
| 130 | /** |
||
| 131 | * The actual line number. |
||
| 132 | * |
||
| 133 | * @var integer |
||
| 134 | */ |
||
| 135 | protected $lineNumber = 0; |
||
| 136 | |||
| 137 | /** |
||
| 138 | * The actual operation name. |
||
| 139 | * |
||
| 140 | * @var string |
||
| 141 | */ |
||
| 142 | protected $operationName ; |
||
| 143 | |||
| 144 | /** |
||
| 145 | * The flag that stop's overserver execution on the actual row. |
||
| 146 | * |
||
| 147 | * @var boolean |
||
| 148 | */ |
||
| 149 | protected $skipRow = false; |
||
| 150 | |||
| 151 | /** |
||
| 152 | * The available EAV attribute sets. |
||
| 153 | * |
||
| 154 | * @var array |
||
| 155 | */ |
||
| 156 | protected $attributeSets = array(); |
||
| 157 | |||
| 158 | /** |
||
| 159 | * The available EAV attributes, grouped by their attribute set and the attribute set name as keys. |
||
| 160 | * |
||
| 161 | * @var array |
||
| 162 | */ |
||
| 163 | protected $attributes = array(); |
||
| 164 | |||
| 165 | /** |
||
| 166 | * The attribute set of the entity that has to be created. |
||
| 167 | * |
||
| 168 | * @var array |
||
| 169 | */ |
||
| 170 | protected $attributeSet = array(); |
||
| 171 | |||
| 172 | /** |
||
| 173 | * The available root categories. |
||
| 174 | * |
||
| 175 | * @var array |
||
| 176 | */ |
||
| 177 | protected $rootCategories = array(); |
||
| 178 | |||
| 179 | /** |
||
| 180 | * The default store. |
||
| 181 | * |
||
| 182 | * @var array |
||
| 183 | */ |
||
| 184 | protected $defaultStore; |
||
| 185 | |||
| 186 | /** |
||
| 187 | * The store view code the create the product/attributes for. |
||
| 188 | * |
||
| 189 | * @var string |
||
| 190 | */ |
||
| 191 | protected $storeViewCode; |
||
| 192 | |||
| 193 | /** |
||
| 194 | * Mappings for attribute code => CSV column header. |
||
| 195 | * |
||
| 196 | * @var array |
||
| 197 | */ |
||
| 198 | protected $headerMappings = array( |
||
| 199 | 'product_online' => 'status', |
||
| 200 | 'tax_class_name' => 'tax_class_id', |
||
| 201 | 'bundle_price_type' => 'price_type', |
||
| 202 | 'bundle_sku_type' => 'sku_type', |
||
| 203 | 'bundle_price_view' => 'price_view', |
||
| 204 | 'bundle_weight_type' => 'weight_type', |
||
| 205 | 'base_image' => 'image', |
||
| 206 | 'base_image_label' => 'image_label', |
||
| 207 | 'thumbnail_image' => 'thumbnail', |
||
| 208 | 'thumbnail_image_label'=> 'thumbnail_label', |
||
| 209 | 'bundle_shipment_type' => 'shipment_type' |
||
| 210 | ); |
||
| 211 | |||
| 212 | /** |
||
| 213 | * Initialize the subject instance. |
||
| 214 | * |
||
| 215 | * @param \Psr\Log\LoggerInterface $systemLogger The system logger instance |
||
| 216 | * @param \TechDivision\Import\Configuration\SubjectConfigurationInterface $configuration The subject configuration instance |
||
| 217 | * @param \TechDivision\Import\Services\RegistryProcessorInterface $registryProcessor The registry processor instance |
||
| 218 | */ |
||
| 219 | public function __construct( |
||
| 228 | |||
| 229 | /** |
||
| 230 | * Stop's observer execution on the actual row. |
||
| 231 | * |
||
| 232 | * @return void |
||
| 233 | */ |
||
| 234 | public function skipRow() |
||
| 238 | |||
| 239 | /** |
||
| 240 | * Return's the actual line number. |
||
| 241 | * |
||
| 242 | * @return integer The line number |
||
| 243 | */ |
||
| 244 | public function getLineNumber() |
||
| 248 | |||
| 249 | /** |
||
| 250 | * Return's the actual operation name. |
||
| 251 | * |
||
| 252 | * @return string |
||
| 253 | */ |
||
| 254 | public function getOperationName() |
||
| 258 | |||
| 259 | /** |
||
| 260 | * Set's the array containing header row. |
||
| 261 | * |
||
| 262 | * @param array $headers The array with the header row |
||
| 263 | * |
||
| 264 | * @return void |
||
| 265 | */ |
||
| 266 | public function setHeaders(array $headers) |
||
| 270 | |||
| 271 | /** |
||
| 272 | * Return's the array containing header row. |
||
| 273 | * |
||
| 274 | * @return array The array with the header row |
||
| 275 | */ |
||
| 276 | public function getHeaders() |
||
| 280 | |||
| 281 | /** |
||
| 282 | * Queries whether or not the header with the passed name is available. |
||
| 283 | * |
||
| 284 | * @param string $name The header name to query |
||
| 285 | * |
||
| 286 | * @return boolean TRUE if the header is available, else FALSE |
||
| 287 | */ |
||
| 288 | public function hasHeader($name) |
||
| 292 | |||
| 293 | /** |
||
| 294 | * Return's the header value for the passed name. |
||
| 295 | * |
||
| 296 | * @param string $name The name of the header to return the value for |
||
| 297 | * |
||
| 298 | * @return mixed The header value |
||
| 299 | * \InvalidArgumentException Is thrown, if the header with the passed name is NOT available |
||
| 300 | */ |
||
| 301 | public function getHeader($name) |
||
| 312 | |||
| 313 | /** |
||
| 314 | * Add's the header with the passed name and position, if not NULL. |
||
| 315 | * |
||
| 316 | * @param string $name The header name to add |
||
| 317 | * |
||
| 318 | * @return integer The new headers position |
||
| 319 | */ |
||
| 320 | public function addHeader($name) |
||
| 329 | |||
| 330 | /** |
||
| 331 | * Queries whether or not debug mode is enabled or not, default is TRUE. |
||
| 332 | * |
||
| 333 | * @return boolean TRUE if debug mode is enabled, else FALSE |
||
| 334 | */ |
||
| 335 | public function isDebugMode() |
||
| 339 | |||
| 340 | /** |
||
| 341 | * Return's the system configuration. |
||
| 342 | * |
||
| 343 | * @return \TechDivision\Import\Configuration\SubjectConfigurationInterface The system configuration |
||
| 344 | */ |
||
| 345 | public function getConfiguration() |
||
| 349 | |||
| 350 | /** |
||
| 351 | * Return's the system logger. |
||
| 352 | * |
||
| 353 | * @return \Psr\Log\LoggerInterface The system logger instance |
||
| 354 | */ |
||
| 355 | public function getSystemLogger() |
||
| 359 | |||
| 360 | /** |
||
| 361 | * Set's root directory for the virtual filesystem. |
||
| 362 | * |
||
| 363 | * @param string $rootDir The root directory for the virtual filesystem |
||
| 364 | * |
||
| 365 | * @return void |
||
| 366 | */ |
||
| 367 | public function setRootDir($rootDir) |
||
| 371 | |||
| 372 | /** |
||
| 373 | * Return's the root directory for the virtual filesystem. |
||
| 374 | * |
||
| 375 | * @return string The root directory for the virtual filesystem |
||
| 376 | */ |
||
| 377 | public function getRootDir() |
||
| 381 | |||
| 382 | /** |
||
| 383 | * Set's the virtual filesystem instance. |
||
| 384 | * |
||
| 385 | * @param \League\Flysystem\FilesystemInterface $filesystem The filesystem instance |
||
| 386 | * |
||
| 387 | * @return void |
||
| 388 | */ |
||
| 389 | public function setFilesystem(FilesystemInterface $filesystem) |
||
| 393 | |||
| 394 | /** |
||
| 395 | * Return's the virtual filesystem instance. |
||
| 396 | * |
||
| 397 | * @return \League\Flysystem\FilesystemInterface The filesystem instance |
||
| 398 | */ |
||
| 399 | public function getFilesystem() |
||
| 403 | |||
| 404 | /** |
||
| 405 | * Return's the RegistryProcessor instance to handle the running threads. |
||
| 406 | * |
||
| 407 | * @return \TechDivision\Import\Services\RegistryProcessorInterface The registry processor instance |
||
| 408 | */ |
||
| 409 | public function getRegistryProcessor() |
||
| 413 | |||
| 414 | /** |
||
| 415 | * Set's the unique serial for this import process. |
||
| 416 | * |
||
| 417 | * @param string $serial The unique serial |
||
| 418 | * |
||
| 419 | * @return void |
||
| 420 | */ |
||
| 421 | public function setSerial($serial) |
||
| 425 | |||
| 426 | /** |
||
| 427 | * Return's the unique serial for this import process. |
||
| 428 | * |
||
| 429 | * @return string The unique serial |
||
| 430 | */ |
||
| 431 | public function getSerial() |
||
| 435 | |||
| 436 | /** |
||
| 437 | * Set's the name of the file to import |
||
| 438 | * |
||
| 439 | * @param string $filename The filename |
||
| 440 | * |
||
| 441 | * @return void |
||
| 442 | */ |
||
| 443 | public function setFilename($filename) |
||
| 447 | |||
| 448 | /** |
||
| 449 | * Return's the name of the file to import. |
||
| 450 | * |
||
| 451 | * @return string The filename |
||
| 452 | */ |
||
| 453 | public function getFilename() |
||
| 457 | |||
| 458 | /** |
||
| 459 | * Return's the source date format to use. |
||
| 460 | * |
||
| 461 | * @return string The source date format |
||
| 462 | */ |
||
| 463 | public function getSourceDateFormat() |
||
| 467 | |||
| 468 | /** |
||
| 469 | * Return's the multiple field delimiter character to use, default value is comma (,). |
||
| 470 | * |
||
| 471 | * @return string The multiple field delimiter character |
||
| 472 | */ |
||
| 473 | public function getMultipleFieldDelimiter() |
||
| 477 | |||
| 478 | /** |
||
| 479 | * Return's the initialized PDO connection. |
||
| 480 | * |
||
| 481 | * @return \PDO The initialized PDO connection |
||
| 482 | */ |
||
| 483 | public function getConnection() |
||
| 487 | |||
| 488 | /** |
||
| 489 | * Intializes the previously loaded global data for exactly one bunch. |
||
| 490 | * |
||
| 491 | * @return void |
||
| 492 | * @see \Importer\Csv\Actions\ProductImportAction::prepare() |
||
| 493 | */ |
||
| 494 | public function setUp() |
||
| 519 | |||
| 520 | /** |
||
| 521 | * This method tries to resolve the passed path and returns it. If the path |
||
| 522 | * is relative, the actual working directory will be prepended. |
||
| 523 | * |
||
| 524 | * @param string $path The path to be resolved |
||
| 525 | * |
||
| 526 | * @return string The resolved path |
||
| 527 | * @throws \InvalidArgumentException Is thrown, if the path can not be resolved |
||
| 528 | */ |
||
| 529 | public function resolvePath($path) |
||
| 546 | |||
| 547 | /** |
||
| 548 | * Clean up the global data after importing the variants. |
||
| 549 | * |
||
| 550 | * @return void |
||
| 551 | */ |
||
| 552 | public function tearDown() |
||
| 569 | |||
| 570 | /** |
||
| 571 | * Return's the next source directory, which will be the target directory |
||
| 572 | * of this subject, in most cases. |
||
| 573 | * |
||
| 574 | * @return string The new source directory |
||
| 575 | */ |
||
| 576 | protected function getNewSourceDir() |
||
| 580 | |||
| 581 | /** |
||
| 582 | * Register the passed observer with the specific type. |
||
| 583 | * |
||
| 584 | * @param \TechDivision\Import\Observers\ObserverInterface $observer The observer to register |
||
| 585 | * @param string $type The type to register the observer with |
||
| 586 | * |
||
| 587 | * @return void |
||
| 588 | */ |
||
| 589 | public function registerObserver(ObserverInterface $observer, $type) |
||
| 601 | |||
| 602 | /** |
||
| 603 | * Register the passed callback with the specific type. |
||
| 604 | * |
||
| 605 | * @param \TechDivision\Import\Callbacks\CallbackInterface $callback The subject to register the callbacks for |
||
| 606 | * @param string $type The type to register the callback with |
||
| 607 | * |
||
| 608 | * @return void |
||
| 609 | */ |
||
| 610 | public function registerCallback(CallbackInterface $callback, $type) |
||
| 622 | |||
| 623 | /** |
||
| 624 | * Return's the array with callbacks for the passed type. |
||
| 625 | * |
||
| 626 | * @param string $type The type of the callbacks to return |
||
| 627 | * |
||
| 628 | * @return array The callbacks |
||
| 629 | */ |
||
| 630 | public function getCallbacksByType($type) |
||
| 644 | |||
| 645 | /** |
||
| 646 | * Return's the array with the available observers. |
||
| 647 | * |
||
| 648 | * @return array The observers |
||
| 649 | */ |
||
| 650 | public function getObservers() |
||
| 654 | |||
| 655 | /** |
||
| 656 | * Return's the array with the available callbacks. |
||
| 657 | * |
||
| 658 | * @return array The callbacks |
||
| 659 | */ |
||
| 660 | public function getCallbacks() |
||
| 664 | |||
| 665 | /** |
||
| 666 | * Return's the callback mappings for this subject. |
||
| 667 | * |
||
| 668 | * @return array The array with the subject's callback mappings |
||
| 669 | */ |
||
| 670 | public function getCallbackMappings() |
||
| 674 | |||
| 675 | /** |
||
| 676 | * Imports the content of the file with the passed filename. |
||
| 677 | * |
||
| 678 | * @param string $serial The unique process serial |
||
| 679 | * @param string $filename The filename to process |
||
| 680 | * |
||
| 681 | * @return void |
||
| 682 | * @throws \Exception Is thrown, if the import can't be processed |
||
| 683 | */ |
||
| 684 | public function import($serial, $filename) |
||
| 769 | |||
| 770 | /** |
||
| 771 | * This method queries whether or not the passed filename matches |
||
| 772 | * the pattern, based on the subjects configured prefix. |
||
| 773 | * |
||
| 774 | * @param string $filename The filename to match |
||
| 775 | * |
||
| 776 | * @return boolean TRUE if the filename matches, else FALSE |
||
| 777 | */ |
||
| 778 | protected function match($filename) |
||
| 787 | |||
| 788 | /** |
||
| 789 | * Initialize and return the lexer configuration. |
||
| 790 | * |
||
| 791 | * @return \Goodby\CSV\Import\Standard\LexerConfig The lexer configuration |
||
| 792 | */ |
||
| 793 | protected function getLexerConfig() |
||
| 827 | |||
| 828 | /** |
||
| 829 | * Imports the passed row into the database. |
||
| 830 | * |
||
| 831 | * If the import failed, the exception will be catched and logged, |
||
| 832 | * but the import process will be continued. |
||
| 833 | * |
||
| 834 | * @param array $row The row with the data to be imported |
||
| 835 | * |
||
| 836 | * @return void |
||
| 837 | */ |
||
| 838 | public function importRow(array $row) |
||
| 878 | |||
| 879 | /** |
||
| 880 | * Map the passed attribute code, if a header mapping exists and return the |
||
| 881 | * mapped mapping. |
||
| 882 | * |
||
| 883 | * @param string $attributeCode The attribute code to map |
||
| 884 | * |
||
| 885 | * @return string The mapped attribute code, or the original one |
||
| 886 | */ |
||
| 887 | public function mapAttributeCodeByHeaderMapping($attributeCode) |
||
| 898 | |||
| 899 | /** |
||
| 900 | * Queries whether or not that the subject needs an OK file to be processed. |
||
| 901 | * |
||
| 902 | * @return boolean TRUE if the subject needs an OK file, else FALSE |
||
| 903 | */ |
||
| 904 | public function isOkFileNeeded() |
||
| 908 | |||
| 909 | /** |
||
| 910 | * Return's the entity type code to be used. |
||
| 911 | * |
||
| 912 | * @return string The entity type code to be used |
||
| 913 | */ |
||
| 914 | public function getEntityTypeCode() |
||
| 918 | |||
| 919 | /** |
||
| 920 | * Set's the attribute set of the product that has to be created. |
||
| 921 | * |
||
| 922 | * @param array $attributeSet The attribute set |
||
| 923 | * |
||
| 924 | * @return void |
||
| 925 | */ |
||
| 926 | public function setAttributeSet(array $attributeSet) |
||
| 930 | |||
| 931 | /** |
||
| 932 | * Return's the attribute set of the product that has to be created. |
||
| 933 | * |
||
| 934 | * @return array The attribute set |
||
| 935 | */ |
||
| 936 | public function getAttributeSet() |
||
| 940 | |||
| 941 | /** |
||
| 942 | * Return's the attribute set with the passed attribute set name. |
||
| 943 | * |
||
| 944 | * @param string $attributeSetName The name of the requested attribute set |
||
| 945 | * |
||
| 946 | * @return array The attribute set data |
||
| 947 | * @throws \Exception Is thrown, if the attribute set with the passed name is not available |
||
| 948 | */ |
||
| 949 | View Code Duplication | public function getAttributeSetByAttributeSetName($attributeSetName) |
|
| 973 | |||
| 974 | /** |
||
| 975 | * Return's the attributes for the attribute set of the product that has to be created. |
||
| 976 | * |
||
| 977 | * @return array The attributes |
||
| 978 | * @throws \Exception Is thrown if the attributes for the actual attribute set are not available |
||
| 979 | */ |
||
| 980 | View Code Duplication | public function getAttributes() |
|
| 1004 | |||
| 1005 | /** |
||
| 1006 | * Return's the EAV attribute with the passed attribute code. |
||
| 1007 | * |
||
| 1008 | * @param string $attributeCode The attribute code |
||
| 1009 | * |
||
| 1010 | * @return array The array with the EAV attribute |
||
| 1011 | * @throws \Exception Is thrown if the attribute with the passed code is not available |
||
| 1012 | */ |
||
| 1013 | public function getEavAttributeByAttributeCode($attributeCode) |
||
| 1034 | |||
| 1035 | /** |
||
| 1036 | * Return's the default store. |
||
| 1037 | * |
||
| 1038 | * @return array The default store |
||
| 1039 | */ |
||
| 1040 | public function getDefaultStore() |
||
| 1044 | |||
| 1045 | /** |
||
| 1046 | * Set's the store view code the create the product/attributes for. |
||
| 1047 | * |
||
| 1048 | * @param string $storeViewCode The store view code |
||
| 1049 | * |
||
| 1050 | * @return void |
||
| 1051 | */ |
||
| 1052 | public function setStoreViewCode($storeViewCode) |
||
| 1056 | |||
| 1057 | /** |
||
| 1058 | * Return's the store view code the create the product/attributes for. |
||
| 1059 | * |
||
| 1060 | * @param string|null $default The default value to return, if the store view code has not been set |
||
| 1061 | * |
||
| 1062 | * @return string The store view code |
||
| 1063 | */ |
||
| 1064 | public function getStoreViewCode($default = null) |
||
| 1078 | |||
| 1079 | /** |
||
| 1080 | * Return's the root category for the actual view store. |
||
| 1081 | * |
||
| 1082 | * @return array The store's root category |
||
| 1083 | * @throws \Exception Is thrown if the root category for the passed store code is NOT available |
||
| 1084 | */ |
||
| 1085 | public function getRootCategory() |
||
| 1102 | } |
||
| 1103 |
If you return a value from a function or method, it should be a sub-type of the type that is given by the parent type f.e. an interface, or abstract method. This is more formally defined by the Lizkov substitution principle, and guarantees that classes that depend on the parent type can use any instance of a child type interchangably. This principle also belongs to the SOLID principles for object oriented design.
Let’s take a look at an example:
Our function
my_functionexpects aPostobject, and outputs the author of the post. The base classPostreturns a simple string and outputting a simple string will work just fine. However, the child classBlogPostwhich is a sub-type ofPostinstead decided to return anobject, and is therefore violating the SOLID principles. If aBlogPostwere passed tomy_function, PHP would not complain, but ultimately fail when executing thestrtouppercall in its body.