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 XoopsObject 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 XoopsObject, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
50 | abstract class XoopsObject implements \ArrayAccess |
||
51 | { |
||
52 | /** |
||
53 | * holds all variables(properties) of an object |
||
54 | * |
||
55 | * @var array |
||
56 | */ |
||
57 | public $vars = array(); |
||
58 | |||
59 | /** |
||
60 | * variables cleaned for store in DB |
||
61 | * |
||
62 | * @var array |
||
63 | */ |
||
64 | public $cleanVars = array(); |
||
65 | |||
66 | /** |
||
67 | * is it a newly created object? |
||
68 | * |
||
69 | * @var bool |
||
70 | */ |
||
71 | private $isNew = false; |
||
72 | |||
73 | /** |
||
74 | * has any of the values been modified? |
||
75 | * |
||
76 | * @var bool |
||
77 | */ |
||
78 | private $isDirty = false; |
||
79 | |||
80 | /** |
||
81 | * errors |
||
82 | * |
||
83 | * @var array |
||
84 | */ |
||
85 | private $errors = array(); |
||
86 | |||
87 | /** |
||
88 | * @var string |
||
89 | */ |
||
90 | public $plugin_path; |
||
91 | |||
92 | /** |
||
93 | * constructor |
||
94 | * |
||
95 | * normally, this is called from child classes only |
||
96 | * |
||
97 | * @access public |
||
98 | */ |
||
99 | 22 | public function __construct() |
|
102 | |||
103 | /** |
||
104 | * used for new/clone objects |
||
105 | * |
||
106 | * @return void |
||
107 | */ |
||
108 | 17 | public function setNew() |
|
112 | |||
113 | /** |
||
114 | * clear new flag |
||
115 | * |
||
116 | * @return void |
||
117 | */ |
||
118 | 8 | public function unsetNew() |
|
122 | |||
123 | /** |
||
124 | * check new flag |
||
125 | * |
||
126 | * @return bool |
||
127 | */ |
||
128 | 12 | public function isNew() |
|
132 | |||
133 | /** |
||
134 | * mark modified objects as dirty |
||
135 | * |
||
136 | * used for modified objects only |
||
137 | * |
||
138 | * @return void |
||
139 | */ |
||
140 | 29 | public function setDirty() |
|
144 | |||
145 | /** |
||
146 | * cleaar dirty flag |
||
147 | * |
||
148 | * @return void |
||
149 | */ |
||
150 | 14 | public function unsetDirty() |
|
154 | |||
155 | /** |
||
156 | * check dirty flag |
||
157 | * |
||
158 | * @return bool |
||
159 | */ |
||
160 | 14 | public function isDirty() |
|
164 | |||
165 | /** |
||
166 | * initialize variables for the object |
||
167 | * |
||
168 | * @param string $key key |
||
169 | * @param int $data_type set to one of Dtype::TYPE_XXX constants (set to Dtype::TYPE_OTHER |
||
170 | * if no data type checking nor text sanitizing is required) |
||
171 | * @param mixed $value value |
||
172 | * @param bool $required require html form input? |
||
173 | * @param mixed $maxlength for Dtype::TYPE_TEXT_BOX type only |
||
174 | * @param string $options does this data have any select options? |
||
175 | * |
||
176 | * @return void |
||
177 | */ |
||
178 | 286 | public function initVar($key, $data_type, $value = null, $required = false, $maxlength = null, $options = '') |
|
189 | |||
190 | /** |
||
191 | * assign a value to a variable |
||
192 | * |
||
193 | * @param string $key name of the variable to assign |
||
194 | * @param mixed $value value to assign |
||
195 | * |
||
196 | * @return void |
||
197 | */ |
||
198 | 54 | public function assignVar($key, $value) |
|
204 | |||
205 | /** |
||
206 | * assign values to multiple variables in a batch |
||
207 | * |
||
208 | * @param array $var_arr associative array of values to assign |
||
209 | * |
||
210 | * @return void |
||
211 | */ |
||
212 | 45 | public function assignVars($var_arr) |
|
220 | |||
221 | /** |
||
222 | * assign a value to a variable |
||
223 | * |
||
224 | * @param string $key name of the variable to assign |
||
225 | * @param mixed $value value to assign |
||
226 | * |
||
227 | * @return void |
||
228 | */ |
||
229 | 27 | public function setVar($key, $value) |
|
237 | |||
238 | /** |
||
239 | * assign values to multiple variables in a batch |
||
240 | * |
||
241 | * @param array $var_arr associative array of values to assign |
||
242 | * |
||
243 | * @return void |
||
244 | */ |
||
245 | 1 | public function setVars($var_arr) |
|
253 | |||
254 | /** |
||
255 | * unset variable(s) for the object |
||
256 | * |
||
257 | * @param mixed $var variable(s) |
||
258 | * |
||
259 | * @return bool |
||
260 | */ |
||
261 | 1 | public function destroyVars($var) |
|
275 | |||
276 | /** |
||
277 | * Assign values to multiple variables in a batch |
||
278 | * |
||
279 | * Meant for a CGI context: |
||
280 | * - prefixed CGI args are considered save |
||
281 | * - avoids polluting of namespace with CGI args |
||
282 | * |
||
283 | * @param mixed $var_arr associative array of values to assign |
||
284 | * @param string $pref prefix (only keys starting with the prefix will be set) |
||
285 | * |
||
286 | * @return void |
||
287 | */ |
||
288 | 1 | public function setFormVars($var_arr = null, $pref = 'xo_') |
|
299 | |||
300 | /** |
||
301 | * returns all variables for the object |
||
302 | * |
||
303 | * @return array associative array of key->value pairs |
||
304 | */ |
||
305 | 26 | public function getVars() |
|
309 | |||
310 | /** |
||
311 | * Returns the values of the specified variables |
||
312 | * |
||
313 | * @param mixed $keys An array containing the names of the keys to retrieve, or null to get all of them |
||
314 | * @param string $format Format to use (see getVar) |
||
315 | * @param int $maxDepth Maximum level of recursion to use if some vars are objects themselves |
||
316 | * |
||
317 | * @return array associative array of key->value pairs |
||
318 | */ |
||
319 | 3 | public function getValues($keys = null, $format = Dtype::FORMAT_SHOW, $maxDepth = 1) |
|
342 | |||
343 | /** |
||
344 | * returns a specific variable for the object in a proper format |
||
345 | * |
||
346 | * @param string $key key of the object's variable to be returned |
||
347 | * @param string $format format to use for the output |
||
348 | * |
||
349 | * @return mixed formatted value of the variable |
||
350 | */ |
||
351 | 186 | public function getVar($key, $format = Dtype::FORMAT_SHOW) |
|
360 | |||
361 | /** |
||
362 | * clean values of all variables of the object for storage. |
||
363 | * |
||
364 | * @return bool true if successful |
||
365 | */ |
||
366 | 3 | public function cleanVars() |
|
384 | |||
385 | /** |
||
386 | * create a clone(copy) of the current object |
||
387 | * |
||
388 | * @return object clone |
||
389 | */ |
||
390 | 1 | public function xoopsClone() |
|
395 | |||
396 | /** |
||
397 | * Adjust a newly cloned object |
||
398 | */ |
||
399 | 1 | public function __clone() |
|
404 | |||
405 | /** |
||
406 | * add an error |
||
407 | * |
||
408 | * @param string $err_str to add |
||
409 | * |
||
410 | * @return void |
||
411 | */ |
||
412 | 2 | public function setErrors($err_str) |
|
420 | |||
421 | /** |
||
422 | * return the errors for this object as an array |
||
423 | * |
||
424 | * @return array an array of errors |
||
425 | */ |
||
426 | 14 | public function getErrors() |
|
430 | |||
431 | /** |
||
432 | * return the errors for this object as html |
||
433 | * |
||
434 | * @return string html listing the errors |
||
435 | * @todo remove hardcoded HTML strings |
||
436 | */ |
||
437 | 1 | public function getHtmlErrors() |
|
449 | |||
450 | /** |
||
451 | * Get object variables as an array |
||
452 | * |
||
453 | * @return array of object values |
||
454 | */ |
||
455 | 1 | public function toArray() |
|
459 | |||
460 | /** |
||
461 | * ArrayAccess methods |
||
462 | */ |
||
463 | |||
464 | /** |
||
465 | * offsetExists |
||
466 | * |
||
467 | * @param mixed $offset array key |
||
468 | * |
||
469 | * @return bool true if offset exists |
||
470 | */ |
||
471 | 2 | public function offsetExists($offset) |
|
475 | |||
476 | /** |
||
477 | * offsetGet |
||
478 | * |
||
479 | * @param mixed $offset array key |
||
480 | * |
||
481 | * @return mixed value |
||
482 | */ |
||
483 | 2 | public function offsetGet($offset) |
|
487 | |||
488 | /** |
||
489 | * offsetSet |
||
490 | * |
||
491 | * @param mixed $offset array key |
||
492 | * @param mixed $value |
||
493 | * |
||
494 | * @return void |
||
495 | */ |
||
496 | 6 | public function offsetSet($offset, $value) |
|
500 | |||
501 | /** |
||
502 | * offsetUnset |
||
503 | * |
||
504 | * @param mixed $offset array key |
||
505 | * |
||
506 | * @return void |
||
507 | */ |
||
508 | 1 | public function offsetUnset($offset) |
|
514 | } |
||
515 |
This check looks for the bodies of
if
statements that have no statements or where all statements have been commented out. This may be the result of changes for debugging or the code may simply be obsolete.These
if
bodies can be removed. If you have an empty if but statements in theelse
branch, consider inverting the condition.could be turned into
This is much more concise to read.