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 DebtorItem 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 DebtorItem, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 17 | class DebtorItem extends Intraface_Standard |
||
| 18 | { |
||
| 19 | /** |
||
| 20 | * @var array |
||
| 21 | */ |
||
| 22 | public $value; |
||
| 23 | |||
| 24 | /** |
||
| 25 | * @var integer |
||
| 26 | */ |
||
| 27 | private $id; |
||
| 28 | |||
| 29 | /** |
||
| 30 | * @var object |
||
| 31 | */ |
||
| 32 | private $invoice; |
||
|
|
|||
| 33 | |||
| 34 | /** |
||
| 35 | * @var object |
||
| 36 | */ |
||
| 37 | private $db; |
||
| 38 | |||
| 39 | /** |
||
| 40 | * @var object |
||
| 41 | */ |
||
| 42 | private $product; |
||
| 43 | |||
| 44 | /** |
||
| 45 | * @var object Intraface_modules_product_Variation |
||
| 46 | */ |
||
| 47 | private $product_variation; |
||
| 48 | |||
| 49 | /** |
||
| 50 | * @var object Intraface_modules_product_Variation_Detail |
||
| 51 | */ |
||
| 52 | private $product_variation_detail; |
||
| 53 | |||
| 54 | /** |
||
| 55 | * @var integer |
||
| 56 | */ |
||
| 57 | private $product_id; |
||
| 58 | |||
| 59 | /** |
||
| 60 | * @var object |
||
| 61 | */ |
||
| 62 | public $error; |
||
| 63 | |||
| 64 | /** |
||
| 65 | * Constructor |
||
| 66 | * |
||
| 67 | * @param object $debtor Debtor object |
||
| 68 | * @param integer $id If a special item id is needed |
||
| 69 | * |
||
| 70 | * @return void |
||
| 71 | */ |
||
| 72 | 68 | View Code Duplication | public function __construct($debtor, $id = 0) |
| 83 | |||
| 84 | /** |
||
| 85 | * Loads data |
||
| 86 | * |
||
| 87 | * @return void |
||
| 88 | */ |
||
| 89 | 1 | private function load() |
|
| 105 | |||
| 106 | /** |
||
| 107 | * returns product object with loaded from item |
||
| 108 | * |
||
| 109 | * @return object Product |
||
| 110 | */ |
||
| 111 | View Code Duplication | private function getProduct() |
|
| 121 | |||
| 122 | /** |
||
| 123 | * Returns Product variation loaded from item |
||
| 124 | * |
||
| 125 | * @return object Intraface_modules_product_Variation |
||
| 126 | */ |
||
| 127 | View Code Duplication | private function getProductVariation() |
|
| 140 | |||
| 141 | /** |
||
| 142 | * Returns product variation detail loaded from item |
||
| 143 | * |
||
| 144 | * @return object Intraface_modules_product_Variation_Detail |
||
| 145 | */ |
||
| 146 | View Code Duplication | private function getProductVariationDetail() |
|
| 159 | |||
| 160 | /** |
||
| 161 | * Returns price of product without vat |
||
| 162 | * |
||
| 163 | * @return float price of product |
||
| 164 | */ |
||
| 165 | View Code Duplication | public function getProductPrice() |
|
| 173 | |||
| 174 | /** |
||
| 175 | * Returns weight of product without vat |
||
| 176 | * |
||
| 177 | * @return integer weight of product |
||
| 178 | */ |
||
| 179 | public function getProductWeight() |
||
| 187 | |||
| 188 | /** |
||
| 189 | * Returns number of product |
||
| 190 | * |
||
| 191 | * @return string product number |
||
| 192 | */ |
||
| 193 | View Code Duplication | public function getProductNumber() |
|
| 201 | |||
| 202 | /** |
||
| 203 | * Returns name of product |
||
| 204 | * |
||
| 205 | * @return string name of product |
||
| 206 | */ |
||
| 207 | View Code Duplication | public function getProductName() |
|
| 215 | |||
| 216 | |||
| 217 | /** |
||
| 218 | * Gets price without taxes |
||
| 219 | * |
||
| 220 | * @return float |
||
| 221 | */ |
||
| 222 | public function getPrice() |
||
| 229 | |||
| 230 | /** |
||
| 231 | * Gets weight |
||
| 232 | * |
||
| 233 | * @return float |
||
| 234 | */ |
||
| 235 | public function getWeight() |
||
| 239 | |||
| 240 | /** |
||
| 241 | * Gets the tax percent on the individual product |
||
| 242 | * |
||
| 243 | * @return float |
||
| 244 | */ |
||
| 245 | public function getTaxPercent() |
||
| 253 | |||
| 254 | /** |
||
| 255 | * Saves data |
||
| 256 | * |
||
| 257 | * @TODO: Should have product object instead of product id in the param array. |
||
| 258 | * |
||
| 259 | * @param array $input Values to save |
||
| 260 | * |
||
| 261 | * @return integer |
||
| 262 | */ |
||
| 263 | 19 | public function save($input) |
|
| 358 | |||
| 359 | /** |
||
| 360 | * Changes the product assigned to the item |
||
| 361 | * |
||
| 362 | * @param integer $product_id |
||
| 363 | * @param integer $product_variation_id |
||
| 364 | * @return boolean true on success |
||
| 365 | */ |
||
| 366 | public function changeProduct($product_id, $product_variation_id = 0) |
||
| 407 | |||
| 408 | /** |
||
| 409 | * Deletes a product |
||
| 410 | * |
||
| 411 | * @return boolean |
||
| 412 | */ |
||
| 413 | public function delete() |
||
| 429 | |||
| 430 | /** |
||
| 431 | * Get a list with Debtor items |
||
| 432 | * |
||
| 433 | * @return array |
||
| 434 | */ |
||
| 435 | 63 | public function getList() |
|
| 436 | { |
||
| 437 | 63 | $db = new DB_Sql; |
|
| 438 | 63 | $db2 = new DB_Sql; |
|
| 439 | |||
| 440 | 63 | $db->query("SELECT id, product_id, product_detail_id, product_variation_id, product_variation_detail_id, quantity, description FROM debtor_item WHERE active = 1 AND intranet_id = ".$this->debtor->kernel->intranet->get("id")." AND debtor_id = ".$this->debtor->get("id")." ORDER BY position ASC, id ASC"); |
|
| 441 | 63 | $i = 0; |
|
| 442 | 63 | $j = 0; |
|
| 443 | 63 | $item_no_vat = array(); |
|
| 444 | 63 | $item_with_vat = array(); |
|
| 445 | |||
| 446 | 63 | require_once 'Intraface/modules/product/Product.php'; |
|
| 447 | |||
| 448 | 63 | $currency = $this->debtor->getCurrency(); |
|
| 449 | |||
| 450 | 63 | while ($db->nextRecord()) { |
|
| 451 | 15 | $product = new Product($this->debtor->kernel, $db->f('product_id'), $db->f('product_detail_id')); |
|
| 452 | |||
| 453 | 15 | if ($product->getId() != 0 && $product->get('detail_id') != 0) { |
|
| 454 | 15 | $item = array(); |
|
| 455 | 15 | $item["description"] = $db->f("description"); |
|
| 456 | 15 | $item["quantity"] = $db->f("quantity"); |
|
| 457 | 15 | $item["id"] = $db->f("id"); |
|
| 458 | |||
| 459 | 15 | $item["vat"] = $product->get("vat"); |
|
| 460 | 15 | $item["product_id"] = $product->getId(); |
|
| 461 | 15 | $item["product_detail_id"] = $product->get("detail_id"); |
|
| 462 | 15 | $unit = $product->get('unit'); |
|
| 463 | 15 | View Code Duplication | if ($db->f("quantity") == 1) { |
| 464 | $item["unit"] = $unit['singular']; |
||
| 465 | } else { |
||
| 466 | 15 | $item["unit"] = $unit['plural']; |
|
| 467 | } |
||
| 468 | |||
| 469 | 15 | if ($product->get('has_variation')) { |
|
| 470 | $variation = $product->getVariation($db->f('product_variation_id')); |
||
| 471 | $detail = $variation->getDetail($db->f('product_variation_detail_id')); |
||
| 472 | $item["name"] = $product->get("name").' - '.$variation->getName(); |
||
| 473 | $item["number"]= $product->get("number").'.'.$variation->getNumber(); |
||
| 474 | $item["price"] = $detail->getPrice($product); |
||
| 475 | if ($currency) { |
||
| 476 | $item['price_currency'] = $detail->getPriceInCurrency($currency, $this->debtor->get('currency_product_price_exchange_rate_id'), $product); |
||
| 477 | } |
||
| 478 | } else { |
||
| 479 | 15 | $item["name"] = $product->get("name"); |
|
| 480 | 15 | $item["number"]= $product->get("number"); |
|
| 481 | 15 | $item["price"] = $product->getDetails()->getPrice(); |
|
| 482 | 15 | if ($currency) { |
|
| 483 | 2 | $item['price_currency'] = $product->getDetails()->getPriceInCurrency($currency, $this->debtor->get('currency_product_price_exchange_rate_id')); |
|
| 484 | 18 | } |
|
| 485 | } |
||
| 486 | |||
| 487 | 15 | if ($product->get("vat") == 0) { |
|
| 488 | 2 | $item_no_vat[$j] = $item; |
|
| 489 | 2 | $item_no_vat[$j]["amount"] = new Ilib_Variable_Float($item["quantity"] * $item["price"]->getAsIso(2)); |
|
| 490 | 2 | View Code Duplication | if ($currency) { |
| 491 | 18 | $item_no_vat[$j]["amount_currency"] = new Ilib_Variable_Float($item["quantity"] * $item["price_currency"]->getAsIso(2), 'iso'); |
|
| 492 | } |
||
| 493 | 2 | $j++; |
|
| 494 | 2 | } else { |
|
| 495 | 13 | $item_with_vat[$i] = $item; |
|
| 496 | 13 | $item_with_vat[$i]["amount"] = new Ilib_Variable_Float($item["quantity"] * $item["price"]->getAsIso(2) * 1.25); |
|
| 497 | 13 | View Code Duplication | if ($currency) { |
| 498 | 2 | $item_with_vat[$i]["amount_currency"] = new Ilib_Variable_Float($item["quantity"] * $item["price_currency"]->getAsIso(2) * 1.25, 'iso'); |
|
| 499 | 2 | } |
|
| 500 | 13 | $i++; |
|
| 501 | } |
||
| 502 | 15 | } else { |
|
| 503 | throw new Exception("Ugyldig produktdetalje i DebtorItem->getList() on ".$db->f('product_id').'/'.$db->f('product_detail_id')); |
||
| 504 | } |
||
| 505 | 15 | } |
|
| 506 | 63 | unset($db); |
|
| 507 | 63 | return(array_merge($item_with_vat, $item_no_vat)); |
|
| 508 | } |
||
| 509 | |||
| 510 | /** |
||
| 511 | * Method to get quantity of each product |
||
| 512 | * |
||
| 513 | * TODO This should not be in this class |
||
| 514 | * |
||
| 515 | * @param integer $product_id Product id |
||
| 516 | * @param string $from_date Which date to start the quantity from |
||
| 517 | * @param string $sent TODO WHAT IS THIS |
||
| 518 | * |
||
| 519 | * @return integer |
||
| 520 | */ |
||
| 521 | 1 | public function getQuantity($product_id, $product_variation_id, $from_date, $sent = "") |
|
| 568 | |||
| 569 | /** |
||
| 570 | * Returns position object |
||
| 571 | * |
||
| 572 | * @return object Ilib_Position |
||
| 573 | */ |
||
| 574 | 18 | function getPosition($db) |
|
| 578 | } |
||
| 579 |
Loading history...
This check marks private properties in classes that are never used. Those properties can be removed.