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 DatabaseHelper 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 DatabaseHelper, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
21 | class DatabaseHelper extends AbstractHelper |
||
22 | { |
||
23 | /** |
||
24 | * @var array|DbSettings |
||
25 | */ |
||
26 | protected $dbSettings = null; |
||
27 | |||
28 | /** |
||
29 | * @var bool |
||
30 | * @deprecated since 1.97.9, use $dbSettings->isSocketConnect() |
||
31 | */ |
||
32 | protected $isSocketConnect = false; |
||
33 | |||
34 | /** |
||
35 | * @var PDO |
||
36 | */ |
||
37 | protected $_connection = null; |
||
38 | |||
39 | /** |
||
40 | * @var array |
||
41 | */ |
||
42 | protected $_tables; |
||
43 | |||
44 | /** |
||
45 | * @param OutputInterface $output |
||
46 | * |
||
47 | * @throws RuntimeException |
||
48 | * @return void |
||
49 | */ |
||
50 | public function detectDbSettings(OutputInterface $output) |
||
74 | |||
75 | /** |
||
76 | * Connects to the database without initializing magento |
||
77 | * |
||
78 | * @param OutputInterface $output = null |
||
79 | * |
||
80 | * @return PDO |
||
81 | */ |
||
82 | public function getConnection(OutputInterface $output = null) |
||
90 | |||
91 | /** |
||
92 | * Creates a PDO DSN for the adapter from $this->_config settings. |
||
93 | * |
||
94 | * @see Zend_Db_Adapter_Pdo_Abstract |
||
95 | * @return string |
||
96 | */ |
||
97 | public function dsn() |
||
101 | |||
102 | /** |
||
103 | * Check whether current mysql user has $privilege privilege |
||
104 | * |
||
105 | * @param string $privilege |
||
106 | * |
||
107 | * @return bool |
||
108 | */ |
||
109 | public function mysqlUserHasPrivilege($privilege) |
||
124 | |||
125 | /** |
||
126 | * @return string |
||
127 | */ |
||
128 | public function getMysqlClientToolConnectionString() |
||
132 | |||
133 | /** |
||
134 | * Get mysql variable value |
||
135 | * |
||
136 | * @param string $variable |
||
137 | * |
||
138 | * @return bool|array returns array on success, false on failure |
||
139 | */ |
||
140 | public function getMysqlVariableValue($variable) |
||
154 | |||
155 | /** |
||
156 | * obtain mysql variable value from the database connection. |
||
157 | * |
||
158 | * in difference to @see getMysqlVariableValue(), this method allows to specify the type of the variable as well |
||
159 | * as to use any variable identifier even such that need quoting. |
||
160 | * |
||
161 | * @param string $name mysql variable name |
||
162 | * @param string $type [optional] variable type, can be a system variable ("@@", default) or a session variable |
||
163 | * ("@"). |
||
164 | * |
||
165 | * @return string variable value, null if variable was not defined |
||
166 | * @throws RuntimeException in case a system variable is unknown (SQLSTATE[HY000]: 1193: Unknown system variable |
||
167 | * 'nonexistent') |
||
168 | */ |
||
169 | public function getMysqlVariable($name, $type = null) |
||
202 | |||
203 | /** |
||
204 | * @param array $commandConfig |
||
205 | * |
||
206 | * @throws RuntimeException |
||
207 | * @return array |
||
208 | */ |
||
209 | public function getTableDefinitions(array $commandConfig) |
||
249 | |||
250 | /** |
||
251 | * @param array $list to resolve |
||
252 | * @param array $definitions from to resolve |
||
253 | * @param array $resolved Which definitions where already resolved -> prevent endless loops |
||
254 | * |
||
255 | * @return array |
||
256 | * @throws RuntimeException |
||
257 | */ |
||
258 | public function resolveTables(array $list, array $definitions = array(), array $resolved = array()) |
||
310 | |||
311 | /** |
||
312 | * @param array $definitions |
||
313 | * @param string $code |
||
314 | * @return array tables |
||
315 | */ |
||
316 | private function resolveRetrieveDefinitionsTablesByCode(array $definitions, $code) |
||
331 | |||
332 | /** |
||
333 | * @param array|null $carry [optional] |
||
334 | * @param $item [optional] |
||
335 | * @return array |
||
336 | * @throws InvalidArgumentException if item is not an array or string |
||
337 | */ |
||
338 | private function resolveTablesArray(array $carry = null, $item = null) |
||
354 | |||
355 | /** |
||
356 | * Get list of database tables |
||
357 | * |
||
358 | * @param bool $withoutPrefix [optional] remove prefix from the returned table names. prefix is obtained from |
||
359 | * magento database configuration. defaults to false. |
||
360 | * |
||
361 | * @return array |
||
362 | * @throws RuntimeException |
||
363 | */ |
||
364 | public function getTables($withoutPrefix = null) |
||
405 | |||
406 | /** |
||
407 | * throw a runtime exception and provide error info for the statement if available |
||
408 | * |
||
409 | * @param PDOStatement $statement |
||
410 | * @param string $message |
||
411 | * |
||
412 | * @throws RuntimeException |
||
413 | */ |
||
414 | private function throwRuntimeException(PDOStatement $statement, $message = "") |
||
428 | |||
429 | /** |
||
430 | * quote a string so that it is safe to use in a LIKE |
||
431 | * |
||
432 | * @param string $string |
||
433 | * @param string $escape character - single us-ascii character |
||
434 | * |
||
435 | * @return string |
||
436 | */ |
||
437 | private function quoteLike($string, $escape = '=') |
||
447 | |||
448 | /** |
||
449 | * Get list of db tables status |
||
450 | * |
||
451 | * @param bool $withoutPrefix |
||
452 | * |
||
453 | * @return array |
||
454 | */ |
||
455 | public function getTablesStatus($withoutPrefix = false) |
||
483 | |||
484 | /** |
||
485 | * @param OutputInterface $output [optional] |
||
486 | * |
||
487 | * @return array|DbSettings |
||
488 | */ |
||
489 | public function getDbSettings(OutputInterface $output = null) |
||
505 | |||
506 | /** |
||
507 | * @return boolean |
||
508 | */ |
||
509 | public function getIsSocketConnect() |
||
513 | |||
514 | /** |
||
515 | * Returns the canonical name of this helper. |
||
516 | * |
||
517 | * @return string The canonical name |
||
518 | * |
||
519 | * @api |
||
520 | */ |
||
521 | public function getName() |
||
525 | |||
526 | /** |
||
527 | * @param OutputInterface $output |
||
528 | */ |
||
529 | View Code Duplication | public function dropDatabase($output) |
|
536 | |||
537 | /** |
||
538 | * @param OutputInterface $output |
||
539 | */ |
||
540 | public function dropTables($output) |
||
553 | |||
554 | /** |
||
555 | * @param OutputInterface $output |
||
556 | */ |
||
557 | View Code Duplication | public function createDatabase($output) |
|
564 | |||
565 | /** |
||
566 | * @param string $command example: 'VARIABLES', 'STATUS' |
||
567 | * @param string|null $variable [optional] |
||
568 | * |
||
569 | * @return array |
||
570 | */ |
||
571 | private function runShowCommand($command, $variable = null) |
||
600 | |||
601 | /** |
||
602 | * @param string|null $variable [optional] |
||
603 | * |
||
604 | * @return array |
||
605 | */ |
||
606 | public function getGlobalVariables($variable = null) |
||
610 | |||
611 | /** |
||
612 | * @param string|null $variable [optional] |
||
613 | * |
||
614 | * @return array |
||
615 | */ |
||
616 | public function getGlobalStatus($variable = null) |
||
620 | |||
621 | /** |
||
622 | * @return Application|BaseApplication |
||
623 | */ |
||
624 | private function getApplication() |
||
636 | |||
637 | /** |
||
638 | * small helper method to obtain an object of type OutputInterface |
||
639 | * |
||
640 | * @param OutputInterface|null $output |
||
641 | * |
||
642 | * @return OutputInterface |
||
643 | */ |
||
644 | private function fallbackOutput(OutputInterface $output = null) |
||
662 | } |
||
663 |
This check marks implicit conversions of arrays to boolean values in a comparison. While in PHP an empty array is considered to be equal (but not identical) to false, this is not always apparent.
Consider making the comparison explicit by using
empty(..)
or! empty(...)
instead.