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 Account 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 Account, and based on these observations, apply Extract Interface, too.
1 | <?php |
||
13 | class Account extends Intraface_Standard |
||
14 | { |
||
15 | /** |
||
16 | * Account id |
||
17 | * |
||
18 | * @var integer |
||
19 | */ |
||
20 | protected $id; |
||
21 | |||
22 | /** |
||
23 | * Year object |
||
24 | * |
||
25 | * @var object |
||
26 | */ |
||
27 | public $year; |
||
28 | |||
29 | /** |
||
30 | * Account values |
||
31 | * |
||
32 | * @var array |
||
33 | */ |
||
34 | public $value; |
||
35 | |||
36 | /** |
||
37 | * Error object |
||
38 | * |
||
39 | * @var object |
||
40 | */ |
||
41 | public $error; |
||
42 | |||
43 | /** |
||
44 | * Vat percent |
||
45 | * |
||
46 | * @var float |
||
47 | */ |
||
48 | public $vat_percent; |
||
49 | |||
50 | /** |
||
51 | * Direction of vat |
||
52 | * |
||
53 | * @var array |
||
54 | */ |
||
55 | public $vat = array( |
||
56 | 0 => 'none', |
||
57 | 1 => 'in', |
||
58 | 2 => 'out' |
||
59 | ); |
||
60 | |||
61 | // Disse b�r laves om til engelske termer med sm�t og s� overs�ttes |
||
62 | // husk at �ndre tilsvarende i validForState() - Status b�r |
||
63 | // splittes op i to konti (aktiver og passiver) |
||
64 | // husk at opdatere databasen til alle sum-konti skal have nummer 5 i stedet |
||
65 | |||
66 | public $types = array( |
||
67 | 1 => 'headline', |
||
68 | 2 => 'operating', // drift |
||
69 | 3 => 'balance, asset', // aktiv |
||
70 | 4 => 'balance, liability', // passiv |
||
71 | 5 => 'sum' |
||
72 | ); |
||
73 | |||
74 | public $use = array( |
||
75 | 1 => 'none', |
||
76 | 2 => 'income', |
||
77 | 3 => 'expenses', |
||
78 | 4 => 'finance' |
||
79 | ); |
||
80 | |||
81 | protected $db; |
||
82 | protected $mdb2; |
||
83 | |||
84 | /** |
||
85 | * Constructor |
||
86 | * |
||
87 | * @param object $year |
||
88 | * @param integer $account_id |
||
89 | * |
||
90 | * @return void |
||
|
|||
91 | */ |
||
92 | 40 | function __construct($year, $account_id = 0) |
|
110 | |||
111 | /** |
||
112 | * Finds an account from number |
||
113 | * |
||
114 | * @deprecated |
||
115 | * @param integer $account_number |
||
116 | * |
||
117 | * @return object |
||
118 | */ |
||
119 | public static function factory($year, $account_number) |
||
124 | |||
125 | /** |
||
126 | * Loads details about an account |
||
127 | * |
||
128 | * @return integer id |
||
129 | */ |
||
130 | 30 | private function load() |
|
204 | |||
205 | /** |
||
206 | * Updateds account |
||
207 | * |
||
208 | * @param array $var info about account |
||
209 | * |
||
210 | * @return integer |
||
211 | */ |
||
212 | 26 | public function save($var) |
|
294 | |||
295 | /** |
||
296 | * Saves the primo saldo |
||
297 | * |
||
298 | * @param float $debet |
||
299 | * @param float $credit |
||
300 | * |
||
301 | * @return boolean |
||
302 | */ |
||
303 | 2 | public function savePrimosaldo($debet, $credit) |
|
332 | |||
333 | /** |
||
334 | * Deletes an account |
||
335 | * |
||
336 | * @todo Skal tjekke om der er poster i �ret p� kontoen. |
||
337 | * |
||
338 | * @return boolean |
||
339 | */ |
||
340 | 1 | public function delete() |
|
356 | |||
357 | /************************************************************************************* |
||
358 | * VALIDERINGSFUNKTIONER |
||
359 | ************************************************************************************/ |
||
360 | |||
361 | /** |
||
362 | * Metoden tjekker om kontoen har den rigtige type, s� vi m� bogf�re p� den. |
||
363 | * |
||
364 | * @return boolean |
||
365 | */ |
||
366 | 11 | public function validForState() |
|
375 | |||
376 | 1 | public function getType() |
|
380 | |||
381 | /** |
||
382 | * Metode til at tjekke om kontonummeret er fri. |
||
383 | * |
||
384 | * @see save() |
||
385 | */ |
||
386 | 26 | private function isNumberFree($account_number) |
|
407 | |||
408 | /************************************************************************************* |
||
409 | * SALDOFUNKTIONER |
||
410 | ************************************************************************************/ |
||
411 | |||
412 | /** |
||
413 | * Public: Metoden returnerer primosaldoen for en konto |
||
414 | * |
||
415 | * @return (array) med debet, credit og total saldo |
||
416 | */ |
||
417 | 5 | function getPrimoSaldo() |
|
438 | /** |
||
439 | * Public: Metoden returnerer en saldo for en konto |
||
440 | * |
||
441 | * Klassen tager h�jde for primobalancen og den skal ogs� tage h�jde for |
||
442 | * sumkonti, se i f�rste omgang getSaldoList(). |
||
443 | * |
||
444 | * Det vil v�re for voldsomt at putte |
||
445 | * den her under get, for s� skal saldoen |
||
446 | * udregnes hver gang jeg skal have fat i |
||
447 | * et eller andet ved en konto! |
||
448 | * |
||
449 | * @param $date_from (date) yyyy-mm-dd Der s�ges jo kun i indev�rende �r |
||
450 | * @param $date_to (date) yyyy-mm-dd Der s�ges kun i indev�rende �r |
||
451 | * |
||
452 | * @return (array) med debet, credit og total saldo |
||
453 | * |
||
454 | * |
||
455 | * |
||
456 | */ |
||
457 | 40 | function getSaldo($type = 'stated', $date_from = '', $date_to = '') |
|
458 | { |
||
459 | 3 | if (empty($date_from)) { |
|
460 | 3 | $date_from = $this->year->get('from_date'); |
|
461 | 3 | } |
|
462 | 3 | if (empty($date_to)) { |
|
463 | 3 | $date_to = $this->year->get('to_date'); |
|
464 | 3 | } |
|
465 | |||
466 | 3 | $total_saldo = 0; |
|
467 | |||
468 | $primo = array( |
||
469 | 3 | 'debet' => '', |
|
470 | 3 | 'credit' => '', |
|
471 | 'saldo' => '' |
||
472 | 3 | ); |
|
473 | |||
474 | // Tjekker p� om datoerne er i indev�rende �r |
||
475 | |||
476 | // henter primosaldoen for kontoen |
||
477 | 3 | $primo = $this->getPrimoSaldo(); |
|
478 | $sql = "SELECT |
||
479 | SUM(post.debet) AS debet_total, |
||
480 | SUM(post.credit) AS credit_total |
||
481 | FROM accounting_post post |
||
482 | INNER JOIN accounting_account account |
||
483 | ON account.id = post.account_id |
||
484 | 40 | WHERE account.id = ".$this->id." |
|
485 | 3 | AND post.year_id = ".$this->year->get('id')." |
|
486 | 3 | AND post.intranet_id = ".$this->year->kernel->intranet->get('id')." |
|
487 | 3 | AND DATE_FORMAT(post.date, '%Y-%m-%d') >= '".$date_from."' |
|
488 | 3 | AND DATE_FORMAT(post.date, '%Y-%m-%d') <= '".$date_to."' |
|
489 | 3 | AND account.year_id = ".$this->year->get('id'); |
|
490 | 3 | if ($type == 'stated') { |
|
491 | 40 | $sql .= ' AND post.stated = 1'; |
|
492 | 3 | } elseif ($type == 'draft') { |
|
493 | $sql .= ' AND post.stated = 0'; |
||
494 | } |
||
495 | |||
496 | 3 | $sql .= " GROUP BY post.account_id"; |
|
497 | |||
498 | 3 | if ($this->get('type_key') == array_search('sum', $this->types)) { |
|
499 | $db2 = new DB_Sql; |
||
500 | $sql = "SELECT id FROM accounting_account |
||
501 | WHERE number >= " . $this->get('sum_from') . " |
||
502 | AND type_key != ".array_search('sum', $this->types)." |
||
503 | AND number <= " . $this->get('sum_to') . " |
||
504 | AND year_id = ".$this->year->get('id')." |
||
505 | AND intranet_id = " . $this->year->kernel->intranet->get('id'); |
||
506 | $db2->query($sql); |
||
507 | $total = 0; |
||
508 | View Code Duplication | while ($db2->nextRecord()) { |
|
509 | // $sub = 0; |
||
510 | $sAccount = new Account($this->year, $db2->f('id')); |
||
511 | $sAccount->getSaldo(); |
||
512 | $total = $total + $sAccount->get('saldo'); |
||
513 | } |
||
514 | $this->value['saldo'] = $total; |
||
515 | $total_saldo = $total_saldo + $total; |
||
516 | } else { |
||
517 | 3 | $this->db->query($sql); |
|
518 | 3 | if (!$this->db->nextRecord()) { |
|
519 | 3 | $this->value['debet'] = $primo['debet']; |
|
520 | 3 | $this->value['credit'] = $primo['credit']; |
|
521 | 3 | $this->value['saldo'] = $this->value['debet'] - $this->value['credit']; |
|
522 | 3 | } else { |
|
523 | if ($type == 'draft') { |
||
524 | $this->value['debet_draft'] = $this->db->f('debet_total'); |
||
525 | $this->value['credit_draft'] = $this->db->f('credit_total'); |
||
526 | $this->value['saldo_draft'] = $this->value['debet_draft'] - $this->value['credit_draft']; |
||
527 | } else { |
||
528 | $this->value['debet'] = $primo['debet'] + $this->db->f('debet_total'); |
||
529 | $this->value['credit'] = $primo['credit'] + $this->db->f('credit_total'); |
||
530 | $this->value['saldo'] = $this->value['debet'] - $this->value['credit']; |
||
531 | } |
||
532 | } |
||
533 | // Det her kan sikkert laves lidt smartere. Den skal egentlig laves inden |
||
534 | // alt det ovenover tror jeg - alst� if-s�tningen |
||
535 | } |
||
536 | |||
537 | 3 | return true; |
|
538 | } |
||
539 | |||
540 | /*************************************************************************** |
||
541 | * �VRIGE METODER |
||
542 | **************************************************************************/ |
||
543 | |||
544 | /** |
||
545 | * Returnerer liste med alle kontoerne |
||
546 | * |
||
547 | * @param string $type Typen af konto, kig i Account::type; |
||
548 | * @param boolean $saldo Whether to return the saldo |
||
549 | * |
||
550 | * @return array |
||
551 | */ |
||
552 | 1 | public function getList($type = '', $saldo = false) |
|
557 | |||
558 | 2 | public function anyAccounts() |
|
563 | |||
564 | 2 | public function anyPosts() |
|
575 | |||
576 | 1 | public function getPosts() |
|
614 | |||
615 | /** |
||
616 | * Calculates the vat amount |
||
617 | * |
||
618 | * @link http://eforum.idg.se/viewmsg.asp?EntriesId=831525 |
||
619 | * |
||
620 | * @param float $amount Amount |
||
621 | * @param float $vat_percent Vat percent |
||
622 | * |
||
623 | * @return float Vat amount |
||
624 | */ |
||
625 | 4 | public function calculateVat($amount, $vat_percent) |
|
632 | |||
633 | public function getId() |
||
634 | { |
||
635 | return $this->id; |
||
636 | } |
||
637 | |||
638 | 1 | public function getNumber() |
|
642 | } |
||
643 |
Adding a
@return
annotation to a constructor is not recommended, since a constructor does not have a meaningful return value.Please refer to the PHP core documentation on constructors.