intraface /
intraface.dk
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 YearEnd 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 YearEnd, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 6 | class YearEnd extends Intraface_Standard |
||
| 7 | { |
||
| 8 | public $error; |
||
| 9 | public $value; |
||
| 10 | public $year; |
||
| 11 | |||
| 12 | /* |
||
|
|
|||
| 13 | protected $step = array( |
||
| 14 | 1 => 'Er alle poster i året indtastet?', |
||
| 15 | 2 => 'Har du lavet momsregnskab og opgivet det til Skat?', |
||
| 16 | 3 => 'Vælg resultatkonto og overfør posterne til resultatkontoen', |
||
| 17 | 4 => 'Rapport med årsregnskabet' |
||
| 18 | ); |
||
| 19 | */ |
||
| 20 | |||
| 21 | // disse typer bruges i forbindelse med om statements er drift eller status |
||
| 22 | public $types = array( |
||
| 23 | 1 => 'operating', |
||
| 24 | 2 => 'balance' |
||
| 25 | ); |
||
| 26 | |||
| 27 | public $actions = array( |
||
| 28 | 1 => 'operating_reset', |
||
| 29 | 2 => 'result_account_reset' |
||
| 30 | ); |
||
| 31 | |||
| 32 | 8 | function __construct($year) |
|
| 43 | |||
| 44 | 8 | View Code Duplication | private function load() |
| 58 | |||
| 59 | 8 | function start() |
|
| 68 | |||
| 69 | 1 | function setStep($step) |
|
| 75 | |||
| 76 | 2 | function setStated($action, $voucher_id) |
|
| 77 | { |
||
| 78 | 2 | $db = new DB_Sql; |
|
| 79 | |||
| 80 | switch ($action) { |
||
| 81 | // bruges i forbindelse med nulstilling af resultatkontoen |
||
| 82 | 2 | case 'operating_reset': |
|
| 83 | 1 | $db->query("UPDATE accounting_year_end SET date_updated=NOW(), operating_reset_voucher_id = " . (int)$voucher_id . " WHERE year_id = " . $this->year->get('id')); |
|
| 84 | 1 | return true; |
|
| 85 | break; |
||
| 86 | // bruges i forbindelse med overførelse af kapitalkontoen |
||
| 87 | 2 | case 'result_account_reset': |
|
| 88 | 2 | $db->query("UPDATE accounting_year_end SET date_updated=NOW(), result_account_reset_voucher_id = " . (int)$voucher_id . " WHERE year_id = " . $this->year->get('id')); |
|
| 89 | 2 | return true; |
|
| 90 | break; |
||
| 91 | |||
| 92 | default: |
||
| 93 | throw new Exception('YearEnd::setStated: Ugyldig type'); |
||
| 94 | break; |
||
| 95 | 2 | } |
|
| 96 | 2 | } |
|
| 97 | |||
| 98 | /** |
||
| 99 | * Denne funktion skal gemme de ændringer der bliver lavet i bogføringen |
||
| 100 | * |
||
| 101 | * Det betyder at vi har mulighed for at vende den igen - skal i så tilfælde bogføres |
||
| 102 | * på samme bilag med modposteringer foran. |
||
| 103 | * |
||
| 104 | * debet-konto |
||
| 105 | * credit-konto |
||
| 106 | * amount |
||
| 107 | * |
||
| 108 | */ |
||
| 109 | function saveStatedAction($action, $voucher_id, $debet_account_id, $credit_account_id, $amount) |
||
| 115 | |||
| 116 | function deleteStatedAction($id) |
||
| 122 | |||
| 123 | 1 | View Code Duplication | function getStatedActions($action) |
| 141 | |||
| 142 | 1 | function flushStatement($type) |
|
| 148 | |||
| 149 | /** |
||
| 150 | * Gemme resultatopgørelsen |
||
| 151 | * |
||
| 152 | * @todo Der er problemer hvis man gemmer resultatopgørelsen flere gange (hvis man også nulstiller kontiene), så det |
||
| 153 | * bør vel egentlig ikke være muligt! Måske kan man forestille sig, at den så bare |
||
| 154 | * gemmer videre og at den ved get lægger tallene i amount sammen? |
||
| 155 | * |
||
| 156 | */ |
||
| 157 | 1 | function saveStatement($type) |
|
| 194 | |||
| 195 | 1 | function getAccount($id = 0) |
|
| 199 | |||
| 200 | function getStatement($type) |
||
| 229 | |||
| 230 | /** |
||
| 231 | * |
||
| 232 | * @param $type (kan være do og reverse) - reverse er hvis man fortryder at man har gemt |
||
| 233 | * dog skal det jo stadig bogføres |
||
| 234 | */ |
||
| 235 | 1 | function resetOperatingAccounts($type = 'do') |
|
| 236 | { |
||
| 237 | switch ($type) { |
||
| 238 | 1 | case 'do': |
|
| 239 | 1 | break; |
|
| 240 | case 'reverse': |
||
| 241 | break; |
||
| 242 | default: |
||
| 243 | throw new Exception('YearEnd::resetOperatingAccounts ugyldig type'); |
||
| 244 | break; |
||
| 245 | } |
||
| 246 | |||
| 247 | 1 | if ($this->year->getSetting('result_account_id') <= 0) { |
|
| 248 | $this->error->set('Resultatkontoen er ikke sat'); |
||
| 249 | } |
||
| 250 | |||
| 251 | 1 | if ($this->error->isError()) { |
|
| 252 | return false; |
||
| 253 | } |
||
| 254 | 1 | $account = $this->getAccount(); |
|
| 255 | 1 | $result_account = $this->getAccount($this->year->getSetting('result_account_id')); |
|
| 256 | |||
| 257 | switch ($type) { |
||
| 258 | 1 | case 'reverse': |
|
| 259 | // hvis man vil reverse skal vi finde actions |
||
| 260 | // vi skal lave bogføringen |
||
| 261 | // og derefter slette actions igen. |
||
| 262 | |||
| 263 | if ($this->get('operating_reset_voucher_id') == 0) { |
||
| 264 | $this->error->set('Du kan ikke lave en reversep� noget der ikke er bogf�rt'); |
||
| 265 | } |
||
| 266 | |||
| 267 | $voucher = new Voucher($this->year, $this->get('operating_reset_voucher_id')); |
||
| 268 | |||
| 269 | $actions = $this->getStatedActions('operating_reset'); |
||
| 270 | |||
| 271 | if (!is_array($actions) or count($actions) == 0) { |
||
| 272 | $this->error->set('Du kan ikke lave en reverse, n�r der ikke er gemt nogen actions'); |
||
| 273 | } |
||
| 274 | |||
| 275 | if ($this->error->isError()) { |
||
| 276 | $this->error->view(); |
||
| 277 | return 0; |
||
| 278 | } |
||
| 279 | |||
| 280 | View Code Duplication | foreach ($actions as $a) { |
|
| 281 | $save_array = array(); |
||
| 282 | // der er byttet om på debet og credit med vilje, fordi |
||
| 283 | // det skal reverses |
||
| 284 | $debet_account = new Account($this->year, $a['credit_account_id']); |
||
| 285 | $credit_account = new Account($this->year, $a['debet_account_id']); |
||
| 286 | |||
| 287 | $save_array = array( |
||
| 288 | 'date' => $this->year->get('to_date_dk'), |
||
| 289 | 'debet_account_number' => $debet_account->get('number'), |
||
| 290 | 'credit_account_number' => $credit_account->get('number'), |
||
| 291 | 'amount' => amountToForm($a['amount']), |
||
| 292 | 'text' => 'Modpostering: ' . $debet_account->get('name') . ' og ' . $credit_account->get('name'), |
||
| 293 | 'vat_off' => 1 |
||
| 294 | |||
| 295 | ); |
||
| 296 | |||
| 297 | if (!empty($save_array)) { |
||
| 298 | if ($voucher->saveInDayBook($save_array, true)) { |
||
| 299 | $this->deleteStatedAction($a['id']); |
||
| 300 | } else { |
||
| 301 | $voucher->error->view(); |
||
| 302 | } |
||
| 303 | } |
||
| 304 | } |
||
| 305 | break; |
||
| 306 | 1 | default: |
|
| 307 | 1 | $voucher = new Voucher($this->year, $this->get('operating_reset_voucher_id')); |
|
| 308 | 1 | $voucher->save(array( |
|
| 309 | 1 | 'date' => $this->year->get('to_date_dk'), |
|
| 310 | 'text' => 'Årsafslutning. Overførsel af driftskonti til resultatopgørelse' |
||
| 311 | 1 | )); |
|
| 312 | |||
| 313 | 1 | $accounts = $account->getList('operating', true); |
|
| 314 | 1 | if (!is_array($accounts) or count($accounts) == 0) { |
|
| 315 | 1 | $this->error->set('Du kan ikke nulstille nogle konti der ikke findes'); |
|
| 316 | 1 | return false; |
|
| 317 | } |
||
| 318 | |||
| 319 | foreach ($accounts as $a) { |
||
| 320 | $save_array = array(); |
||
| 321 | $account = new Account($this->year, $a['id']); |
||
| 322 | $account->getSaldo(); |
||
| 323 | |||
| 324 | View Code Duplication | if ($account->get('saldo') > 0) { |
|
| 325 | $save_array = array( |
||
| 326 | 'date' => $this->year->get('to_date_dk'), |
||
| 327 | 'debet_account_number' => $result_account->get('number'), |
||
| 328 | 'credit_account_number' => $account->get('number'), |
||
| 329 | 'amount' => amountToForm(abs($account->get('saldo'))), // amountToFrom necessary to get the correct format for daybook |
||
| 330 | 'text' => $account->get('name') . ' til resultatkontoen', |
||
| 331 | 'vat_off' => 1 |
||
| 332 | |||
| 333 | ); |
||
| 334 | $debet_account = $result_account; |
||
| 335 | $credit_account = $account; |
||
| 336 | } elseif ($account->get('saldo') <= 0) { |
||
| 337 | $save_array = array( |
||
| 338 | 'date' => $this->year->get('to_date_dk'), |
||
| 339 | 'debet_account_number' => $account->get('number'), |
||
| 340 | 'credit_account_number' => $result_account->get('number'), |
||
| 341 | 'amount' => amountToForm(abs($account->get('saldo'))), // amountToFrom necessary to get the correct format for daybook |
||
| 342 | 'text' => $account->get('name') . ' til resultatkontoen', |
||
| 343 | 'vat_off' => 1 |
||
| 344 | ); |
||
| 345 | $debet_account = $account; |
||
| 346 | $credit_account = $result_account; |
||
| 347 | } |
||
| 348 | |||
| 349 | View Code Duplication | if (!empty($save_array)) { |
|
| 350 | if ($voucher->saveInDayBook($save_array, true)) { |
||
| 351 | $this->saveStatedAction('operating_reset', $voucher->get('id'), $debet_account->get('id'), $credit_account->get('id'), abs(amountToForm($account->get('saldo')))); |
||
| 352 | } |
||
| 353 | } |
||
| 354 | $this->setStated('operating_reset', $voucher->get('id')); |
||
| 355 | } |
||
| 356 | return true; |
||
| 357 | break; |
||
| 358 | 1 | } |
|
| 359 | } |
||
| 360 | |||
| 361 | 1 | function resetYearResult($type = 'do') |
|
| 489 | } |
||
| 490 |
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.