mambax7 /
smartfaq
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 AnswerHandler 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 AnswerHandler, and based on these observations, apply Extract Interface, too.
| 1 | <?php namespace XoopsModules\Smartfaq; |
||
| 23 | class AnswerHandler extends \XoopsPersistableObjectHandler |
||
| 24 | { |
||
| 25 | |||
| 26 | /** |
||
| 27 | * create a new answer |
||
| 28 | * |
||
| 29 | * @param bool $isNew flag the new objects as "new"? |
||
| 30 | * @return object Answer |
||
| 31 | */ |
||
| 32 | public function create($isNew = true) |
||
| 41 | |||
| 42 | /** |
||
| 43 | * retrieve an answer |
||
| 44 | * |
||
| 45 | * @param int $id answerid of the answer |
||
| 46 | * @param null $fields |
||
| 47 | * @return mixed reference to the <a href='psi_element://sfAnswer'>sfAnswer</a> object, FALSE if failed |
||
| 48 | */ |
||
| 49 | View Code Duplication | public function get($id = null, $fields = null) |
|
| 68 | |||
| 69 | /** |
||
| 70 | * insert a new answer in the database |
||
| 71 | * |
||
| 72 | * @param \XoopsObject $answerObj reference to the <a href='psi_element://sfAnswer'>sfAnswer</a> object |
||
| 73 | * @param bool $force |
||
| 74 | * @return bool FALSE if failed, TRUE if already present and unchanged or successful |
||
| 75 | */ |
||
| 76 | public function insert(\XoopsObject $answerObj, $force = false) |
||
| 116 | |||
| 117 | /** |
||
| 118 | * delete an answer from the database |
||
| 119 | * |
||
| 120 | * @param \XoopsObject $answer reference to the answer to delete |
||
| 121 | * @param bool $force |
||
| 122 | * @return bool FALSE if failed. |
||
| 123 | */ |
||
| 124 | public function delete(\XoopsObject $answer, $force = false) |
||
| 144 | |||
| 145 | /** |
||
| 146 | * delete an answer from the database |
||
| 147 | * |
||
| 148 | * @param object $faqObj reference to the answer to delete |
||
| 149 | * @return bool FALSE if failed. |
||
| 150 | * @internal param bool $force |
||
| 151 | */ |
||
| 152 | public function deleteFaqAnswers($faqObj) |
||
| 167 | |||
| 168 | /** |
||
| 169 | * retrieve answers from the database |
||
| 170 | * |
||
| 171 | * @param \CriteriaElement $criteria {@link CriteriaElement} conditions to be met |
||
| 172 | * @param bool $id_as_key use the answerid as key for the array? |
||
| 173 | * @param bool $as_object |
||
| 174 | * @return array array of <a href='psi_element://sfAnswer'>sfAnswer</a> objects |
||
| 175 | */ |
||
| 176 | View Code Duplication | public function &getObjects(\CriteriaElement $criteria = null, $id_as_key = false, $as_object = true) |
|
| 207 | |||
| 208 | /** |
||
| 209 | * retrieve 1 official answer (for now SmartFAQ only allow 1 official answer...) |
||
| 210 | * |
||
| 211 | * @param int $faqid |
||
| 212 | * @return mixed reference to the <a href='psi_element://sfAnswer'>sfAnswer</a> object, FALSE if failed |
||
| 213 | */ |
||
| 214 | public function getOfficialAnswer($faqid = 0) |
||
| 224 | |||
| 225 | /** |
||
| 226 | * retrieve all answers |
||
| 227 | * |
||
| 228 | * @param int $faqid |
||
| 229 | * @param int $status |
||
| 230 | * @param int $limit |
||
| 231 | * @param int $start |
||
| 232 | * @param string $sort |
||
| 233 | * @param string $order |
||
| 234 | * @return array array of <a href='psi_element://sfAnswer'>sfAnswer</a> objects |
||
| 235 | */ |
||
| 236 | public function getAllAnswers( |
||
| 272 | |||
| 273 | /** |
||
| 274 | * count answers matching a condition |
||
| 275 | * |
||
| 276 | * @param \CriteriaElement $criteria {@link CriteriaElement} to match |
||
| 277 | * @return int count of answers |
||
| 278 | */ |
||
| 279 | View Code Duplication | public function getCount(\CriteriaElement $criteria = null) |
|
| 293 | |||
| 294 | /** |
||
| 295 | * count answers matching a condition and group by faq ID |
||
| 296 | * |
||
| 297 | * @param object $criteria {@link CriteriaElement} to match |
||
| 298 | * @return array |
||
| 299 | */ |
||
| 300 | public function getCountByFAQ($criteria = null) |
||
| 321 | |||
| 322 | /** |
||
| 323 | * delete answers matching a set of conditions |
||
| 324 | * |
||
| 325 | * @param \CriteriaElement $criteria {@link CriteriaElement} |
||
| 326 | * @param bool $force |
||
| 327 | * @param bool $asObject |
||
| 328 | * @return bool FALSE if deletion failed |
||
| 329 | */ |
||
| 330 | public function deleteAll(\CriteriaElement $criteria = null, $force = true, $asObject = false) |
||
| 342 | |||
| 343 | /** |
||
| 344 | * Change a value for answers with a certain criteria |
||
| 345 | * |
||
| 346 | * @param string $fieldname Name of the field |
||
| 347 | * @param string $fieldvalue Value to write |
||
| 348 | * @param \CriteriaElement $criteria {@link CriteriaElement} |
||
| 349 | * @param bool $force |
||
| 350 | * @return bool |
||
| 351 | */ |
||
| 352 | View Code Duplication | public function updateAll($fieldname, $fieldvalue, \CriteriaElement $criteria = null, $force = false) |
|
| 366 | |||
| 367 | /** |
||
| 368 | * @param $faqids |
||
| 369 | * @return array |
||
| 370 | */ |
||
| 371 | public function getLastPublishedByFaq($faqids) |
||
| 389 | } |
||
| 390 |
Loading history...
Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.
The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.
This check looks for comments that seem to be mostly valid code and reports them.