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 DBRow 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 DBRow, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
24 | abstract class DBRow extends PropertyHiderInObject implements IDbRow { |
||
25 | // TODO |
||
26 | /** |
||
27 | * Should be this object - (!) not class - cached |
||
28 | * There exists tables that didn't need to cache rows - logs as example |
||
29 | * And there can be special needs to not cache some class instances when stream-reading many rows i.e. fleets in stat calculation |
||
30 | * |
||
31 | * @var bool $_cacheable |
||
32 | */ |
||
33 | public $_cacheable = true; // |
||
34 | // TODO |
||
35 | /** |
||
36 | * БД для доступа к данным |
||
37 | * |
||
38 | * @var db_mysql $db |
||
39 | */ |
||
40 | protected static $db = null; |
||
41 | /** |
||
42 | * Table name in DB |
||
43 | * |
||
44 | * @var string |
||
45 | */ |
||
46 | protected static $_table = ''; |
||
47 | /** |
||
48 | * Name of ID field in DB |
||
49 | * |
||
50 | * @var string |
||
51 | */ |
||
52 | protected static $_dbIdFieldName = 'id'; |
||
53 | |||
54 | /** |
||
55 | * DB_ROW to Class translation scheme |
||
56 | * |
||
57 | * @var array |
||
58 | */ |
||
59 | protected $_properties = array( |
||
60 | 'dbId' => array( |
||
61 | P_DB_FIELD => 'id', |
||
62 | ), |
||
63 | ); |
||
64 | |||
65 | /** |
||
66 | * Object list that should mimic object DB operations - i.e. units on fleet |
||
67 | * |
||
68 | * @var IDbRow[] |
||
69 | */ |
||
70 | protected $triggerDbOperationOn = array(); // Not a static - because it's an object array |
||
71 | |||
72 | /** |
||
73 | * @var int |
||
74 | */ |
||
75 | protected $_dbId = 0; |
||
76 | |||
77 | /** |
||
78 | * Flag to skip lock on current Load operation |
||
79 | * |
||
80 | * @var bool |
||
81 | */ |
||
82 | protected $lockSkip = false; |
||
83 | |||
84 | |||
85 | /** |
||
86 | * @param db_mysql|null $db |
||
87 | */ |
||
88 | 1 | public static function setDb($db = null) { |
|
94 | |||
95 | 2 | public static function getDb() { |
|
98 | |||
99 | /** |
||
100 | * @param string $tableName |
||
101 | */ |
||
102 | 1 | public static function setTable($tableName) { |
|
105 | |||
106 | 1 | public static function getTable() { |
|
109 | |||
110 | /** |
||
111 | * @param string $dbIdFieldName |
||
112 | */ |
||
113 | 1 | public static function setIdFieldName($dbIdFieldName) { |
|
116 | |||
117 | 1 | public static function getIdFieldName() { |
|
120 | |||
121 | // Some magic ******************************************************************************************************** |
||
122 | |||
123 | /** |
||
124 | * DBRow constructor. |
||
125 | * |
||
126 | * @param db_mysql|null $db |
||
127 | */ |
||
128 | 1 | public function __construct($db = null) { |
|
136 | |||
137 | |||
138 | |||
139 | // IDBrow Implementation ********************************************************************************************* |
||
140 | |||
141 | /** |
||
142 | * Loading object from DB by primary ID |
||
143 | * |
||
144 | * @param int $dbId |
||
145 | * @param bool $lockSkip |
||
146 | * |
||
147 | * @return |
||
148 | */ |
||
149 | public function dbLoad($dbId, $lockSkip = false) { |
||
150 | $dbId = idval($dbId); |
||
151 | if ($dbId <= 0) { |
||
152 | classSupernova::$debug->error(get_called_class() . '::' . __METHOD__ . ' $dbId not positive = ' . $dbId); |
||
153 | |||
154 | return; |
||
155 | } |
||
156 | |||
157 | $this->_dbId = $dbId; |
||
|
|||
158 | $this->lockSkip = $lockSkip; |
||
159 | // TODO - Use classSupernova::$db_records_locked |
||
160 | if (false && !$lockSkip && sn_db_transaction_check(false)) { |
||
161 | $this->dbGetLockById($this->_dbId); |
||
162 | } |
||
163 | |||
164 | $db_row = classSupernova::$db->doSelectFetch("SELECT * FROM `{{" . static::$_table . "}}` WHERE `" . static::$_dbIdFieldName . "` = " . $this->_dbId . " LIMIT 1 FOR UPDATE;"); |
||
165 | if (empty($db_row)) { |
||
166 | return; |
||
167 | } |
||
168 | |||
169 | $this->dbRowParse($db_row); |
||
170 | $this->lockSkip = false; |
||
171 | } |
||
172 | |||
173 | /** |
||
174 | * Lock all fields that belongs to operation |
||
175 | * |
||
176 | * @param int $dbId |
||
177 | * |
||
178 | * @return |
||
179 | * param DBLock $dbRow - Object that accumulates locks |
||
180 | * |
||
181 | */ |
||
182 | abstract public function dbGetLockById($dbId); |
||
183 | |||
184 | /** |
||
185 | * Saving object to DB |
||
186 | * This is meta-method: |
||
187 | * - if object is new - then it inserted to DB; |
||
188 | * - if object is empty - it deleted from DB; |
||
189 | * - otherwise object is updated in DB; |
||
190 | */ |
||
191 | // TODO - perform operations only if properties was changed |
||
192 | public function dbSave() { |
||
220 | |||
221 | |||
222 | |||
223 | // CRUD ************************************************************************************************************** |
||
224 | |||
225 | /** |
||
226 | * Inserts record to DB |
||
227 | * |
||
228 | * @return int|string |
||
229 | */ |
||
230 | // TODO - protected |
||
231 | public function dbInsert() { |
||
245 | |||
246 | /** |
||
247 | * Updates record in DB |
||
248 | */ |
||
249 | // TODO - protected |
||
250 | public function dbUpdate() { |
||
257 | |||
258 | /** |
||
259 | * Deletes record from DB |
||
260 | */ |
||
261 | // TODO - protected |
||
262 | public function dbDelete() { |
||
270 | |||
271 | /** |
||
272 | * Является ли запись новой - т.е. не имеет своей записи в БД |
||
273 | * |
||
274 | * @return bool |
||
275 | */ |
||
276 | public function isNew() { |
||
279 | |||
280 | /** |
||
281 | * Является ли запись пустой - т.е. при исполнении _dbSave должен быть удалён |
||
282 | * |
||
283 | * @return bool |
||
284 | */ |
||
285 | abstract public function isEmpty(); |
||
286 | |||
287 | // Other Methods ***************************************************************************************************** |
||
288 | /** |
||
289 | * Парсит запись из БД в поля объекта |
||
290 | * |
||
291 | * @param array $db_row |
||
292 | */ |
||
293 | public function dbRowParse(array $db_row) { |
||
323 | |||
324 | /** |
||
325 | * Делает из свойств класса массив db_field_name => db_field_value |
||
326 | * |
||
327 | * @return array |
||
328 | */ |
||
329 | protected function dbMakeFieldSet($isUpdate = false) { |
||
367 | |||
368 | /** |
||
369 | * Делает из свойств класса массив db_field_name => db_field_value |
||
370 | * @return array |
||
371 | * @deprecated |
||
372 | */ |
||
373 | protected function dbMakeFieldUpdate() { |
||
403 | |||
404 | /** |
||
405 | * Check if DB field changed on property change and if it changed - returns name of property which triggered change |
||
406 | * |
||
407 | * @param string $fieldName |
||
408 | * |
||
409 | * @return string|false |
||
410 | */ |
||
411 | protected function isFieldChanged($fieldName) { |
||
431 | |||
432 | /** |
||
433 | * @param array $field_set |
||
434 | * |
||
435 | * @return array|bool|mysqli_result|null |
||
436 | * @deprecated |
||
437 | */ |
||
438 | protected function db_field_update(array $field_set) { |
||
463 | |||
464 | } |
||
465 |
Our type inference engine has found a suspicous assignment of a value to a property. This check raises an issue when a value that can be of a mixed type is assigned to a property that is type hinted more strictly.
For example, imagine you have a variable
$accountId
that can either hold an Id object or false (if there is no account id yet). Your code now assigns that value to theid
property of an instance of theAccount
class. This class holds a proper account, so the id value must no longer be false.Either this assignment is in error or a type check should be added for that assignment.