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 |
||
28 | class DbRow |
||
29 | { |
||
30 | /** |
||
31 | * The service this object is bound to. |
||
32 | * |
||
33 | * @var TDBMService |
||
34 | */ |
||
35 | protected $tdbmService; |
||
36 | |||
37 | /** |
||
38 | * The object containing this db row. |
||
39 | * |
||
40 | * @var AbstractTDBMObject |
||
41 | */ |
||
42 | private $object; |
||
43 | |||
44 | /** |
||
45 | * The name of the table the object if issued from. |
||
46 | * |
||
47 | * @var string |
||
48 | */ |
||
49 | private $dbTableName; |
||
50 | |||
51 | /** |
||
52 | * The array of columns returned from database. |
||
53 | * |
||
54 | * @var array |
||
55 | */ |
||
56 | private $dbRow = array(); |
||
57 | |||
58 | /** |
||
59 | * @var AbstractTDBMObject[] |
||
60 | */ |
||
61 | private $references = array(); |
||
62 | |||
63 | /** |
||
64 | * One of TDBMObjectStateEnum::STATE_NEW, TDBMObjectStateEnum::STATE_NOT_LOADED, TDBMObjectStateEnum::STATE_LOADED, TDBMObjectStateEnum::STATE_DELETED. |
||
65 | * $status = TDBMObjectStateEnum::STATE_NEW when a new object is created with DBMObject:getNewObject. |
||
66 | * $status = TDBMObjectStateEnum::STATE_NOT_LOADED when the object has been retrieved with getObject but when no data has been accessed in it yet. |
||
67 | * $status = TDBMObjectStateEnum::STATE_LOADED when the object is cached in memory. |
||
68 | * |
||
69 | * @var string |
||
70 | */ |
||
71 | private $status; |
||
72 | |||
73 | /** |
||
74 | * The values of the primary key. |
||
75 | * This is set when the object is in "loaded" state. |
||
76 | * |
||
77 | * @var array An array of column => value |
||
78 | */ |
||
79 | private $primaryKeys; |
||
80 | |||
81 | /** |
||
82 | * You should never call the constructor directly. Instead, you should use the |
||
83 | * TDBMService class that will create TDBMObjects for you. |
||
84 | * |
||
85 | * Used with id!=false when we want to retrieve an existing object |
||
86 | * and id==false if we want a new object |
||
87 | * |
||
88 | * @param AbstractTDBMObject $object The object containing this db row |
||
89 | * @param string $table_name |
||
90 | * @param array $primaryKeys |
||
91 | * @param TDBMService $tdbmService |
||
92 | * |
||
93 | * @throws TDBMException |
||
94 | * @throws TDBMInvalidOperationException |
||
95 | */ |
||
96 | public function __construct(AbstractTDBMObject $object, $table_name, array $primaryKeys = array(), TDBMService $tdbmService = null, array $dbRow = array()) |
||
125 | |||
126 | public function _attach(TDBMService $tdbmService) |
||
135 | |||
136 | /** |
||
137 | * Sets the state of the TDBM Object |
||
138 | * One of TDBMObjectStateEnum::STATE_NEW, TDBMObjectStateEnum::STATE_NOT_LOADED, TDBMObjectStateEnum::STATE_LOADED, TDBMObjectStateEnum::STATE_DELETED. |
||
139 | * $status = TDBMObjectStateEnum::STATE_NEW when a new object is created with DBMObject:getNewObject. |
||
140 | * $status = TDBMObjectStateEnum::STATE_NOT_LOADED when the object has been retrieved with getObject but when no data has been accessed in it yet. |
||
141 | * $status = TDBMObjectStateEnum::STATE_LOADED when the object is cached in memory. |
||
142 | * |
||
143 | * @param string $state |
||
144 | */ |
||
145 | public function _setStatus($state) |
||
149 | |||
150 | /** |
||
151 | * This is an internal method. You should not call this method yourself. The TDBM library will do it for you. |
||
152 | * If the object is in state 'not loaded', this method performs a query in database to load the object. |
||
153 | * |
||
154 | * A TDBMException is thrown is no object can be retrieved (for instance, if the primary key specified |
||
155 | * cannot be found). |
||
156 | */ |
||
157 | public function _dbLoadIfNotLoaded() |
||
185 | |||
186 | public function get($var) |
||
192 | |||
193 | /** |
||
194 | * Returns true if a column is set, false otherwise. |
||
195 | * |
||
196 | * @param string $var |
||
197 | * |
||
198 | * @return bool |
||
199 | */ |
||
200 | /*public function has($var) { |
||
201 | $this->_dbLoadIfNotLoaded(); |
||
202 | |||
203 | return isset($this->dbRow[$var]); |
||
204 | }*/ |
||
205 | |||
206 | View Code Duplication | public function set($var, $value) |
|
228 | |||
229 | /** |
||
230 | * @param string $foreignKeyName |
||
231 | * @param AbstractTDBMObject $bean |
||
232 | */ |
||
233 | View Code Duplication | public function setRef($foreignKeyName, AbstractTDBMObject $bean = null) |
|
242 | |||
243 | /** |
||
244 | * @param string $foreignKeyName A unique name for this reference |
||
245 | * |
||
246 | * @return AbstractTDBMObject|null |
||
247 | */ |
||
248 | public function getRef($foreignKeyName) |
||
279 | |||
280 | /** |
||
281 | * Returns the name of the table this object comes from. |
||
282 | * |
||
283 | * @return string |
||
284 | */ |
||
285 | public function _getDbTableName() |
||
289 | |||
290 | /** |
||
291 | * Method used internally by TDBM. You should not use it directly. |
||
292 | * This method returns the status of the TDBMObject. |
||
293 | * This is one of TDBMObjectStateEnum::STATE_NEW, TDBMObjectStateEnum::STATE_NOT_LOADED, TDBMObjectStateEnum::STATE_LOADED, TDBMObjectStateEnum::STATE_DELETED. |
||
294 | * $status = TDBMObjectStateEnum::STATE_NEW when a new object is created with DBMObject:getNewObject. |
||
295 | * $status = TDBMObjectStateEnum::STATE_NOT_LOADED when the object has been retrieved with getObject but when no data has been accessed in it yet. |
||
296 | * $status = TDBMObjectStateEnum::STATE_LOADED when the object is cached in memory. |
||
297 | * |
||
298 | * @return string |
||
299 | */ |
||
300 | public function _getStatus() |
||
304 | |||
305 | /** |
||
306 | * Override the native php clone function for TDBMObjects. |
||
307 | */ |
||
308 | public function __clone() |
||
326 | |||
327 | /** |
||
328 | * Returns raw database row. |
||
329 | * |
||
330 | * @return array |
||
331 | * |
||
332 | * @throws TDBMMissingReferenceException |
||
333 | */ |
||
334 | public function _getDbRow() |
||
363 | |||
364 | /** |
||
365 | * Returns references array. |
||
366 | * |
||
367 | * @return AbstractTDBMObject[] |
||
368 | */ |
||
369 | public function _getReferences() |
||
373 | |||
374 | /** |
||
375 | * Returns the values of the primary key. |
||
376 | * This is set when the object is in "loaded" state. |
||
377 | * |
||
378 | * @return array |
||
379 | */ |
||
380 | public function _getPrimaryKeys() |
||
384 | |||
385 | /** |
||
386 | * Sets the values of the primary key. |
||
387 | * This is set when the object is in "loaded" state. |
||
388 | * |
||
389 | * @param array $primaryKeys |
||
390 | */ |
||
391 | public function _setPrimaryKeys(array $primaryKeys) |
||
398 | |||
399 | /** |
||
400 | * Returns the TDBMObject this bean is associated to. |
||
401 | * |
||
402 | * @return AbstractTDBMObject |
||
403 | */ |
||
404 | public function getTDBMObject() |
||
408 | |||
409 | /** |
||
410 | * Sets the TDBMObject this bean is associated to. |
||
411 | * Only used when cloning. |
||
412 | * |
||
413 | * @param AbstractTDBMObject $object |
||
414 | */ |
||
415 | public function setTDBMObject(AbstractTDBMObject $object) |
||
419 | } |
||
420 |
Instead of embedding dynamic parameters in SQL, Doctrine also allows you to pass them separately and insert a placeholder instead: