laudeco /
bbc-dolibarr-flightlog
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 Bbcvols 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 Bbcvols, and based on these observations, apply Extract Interface, too.
| 1 | <?php |
||
| 41 | class Bbcvols extends CommonObject |
||
| 42 | { |
||
| 43 | /** |
||
| 44 | * @var string Id to identify managed objects |
||
| 45 | */ |
||
| 46 | public $element = 'flightlog_bbcvols'; |
||
| 47 | /** |
||
| 48 | * @var string Name of table without prefix where object is stored |
||
| 49 | */ |
||
| 50 | public $table_element = 'bbc_vols'; |
||
| 51 | |||
| 52 | /** |
||
| 53 | * @var BbcvolsLine[] Lines |
||
| 54 | */ |
||
| 55 | public $lines = array(); |
||
| 56 | |||
| 57 | public $idBBC_vols; |
||
| 58 | public $date = ''; |
||
| 59 | public $lieuD; |
||
| 60 | public $lieuA; |
||
| 61 | public $heureD; |
||
| 62 | public $heureA; |
||
| 63 | public $BBC_ballons_idBBC_ballons; |
||
| 64 | public $nbrPax; |
||
| 65 | public $remarque; |
||
| 66 | public $incidents; |
||
| 67 | public $fk_type; |
||
| 68 | public $fk_pilot; |
||
| 69 | public $fk_organisateur; |
||
| 70 | public $is_facture; |
||
| 71 | public $kilometers; |
||
| 72 | public $cost; |
||
| 73 | public $fk_receiver; |
||
| 74 | public $justif_kilometers; |
||
| 75 | public $date_creation; |
||
| 76 | public $date_update; |
||
| 77 | |||
| 78 | /** |
||
| 79 | * @var Bbc_ballons |
||
| 80 | */ |
||
| 81 | private $balloon; |
||
| 82 | |||
| 83 | /** |
||
| 84 | * @var User |
||
| 85 | */ |
||
| 86 | private $pilot; |
||
| 87 | |||
| 88 | /** |
||
| 89 | * @return int |
||
| 90 | */ |
||
| 91 | public function getIdBBCVols() |
||
| 95 | |||
| 96 | /** |
||
| 97 | * @return int |
||
| 98 | */ |
||
| 99 | public function getId() |
||
| 103 | |||
| 104 | /** |
||
| 105 | * @param string|int $ref |
||
| 106 | * |
||
| 107 | * @return $this |
||
| 108 | */ |
||
| 109 | public function setRef($ref) |
||
| 114 | |||
| 115 | /** |
||
| 116 | * Constructor |
||
| 117 | * |
||
| 118 | * @param DoliDb $db Database handler |
||
| 119 | */ |
||
| 120 | public function __construct(DoliDB $db) |
||
| 125 | |||
| 126 | /** |
||
| 127 | * Create object into database |
||
| 128 | * |
||
| 129 | * @param User $user User that creates |
||
| 130 | * @param bool $notrigger false=launch triggers after, true=disable triggers |
||
| 131 | * |
||
| 132 | * @return int <0 if KO, Id of created object if OK |
||
| 133 | */ |
||
| 134 | public function create(User $user, $notrigger = false) |
||
| 135 | { |
||
| 136 | dol_syslog(__METHOD__, LOG_DEBUG); |
||
| 137 | |||
| 138 | $error = 0; |
||
| 139 | |||
| 140 | // Clean parameters |
||
| 141 | |||
| 142 | if (isset($this->idBBC_vols)) { |
||
| 143 | $this->idBBC_vols = trim($this->idBBC_vols); |
||
| 144 | } |
||
| 145 | if (isset($this->lieuD)) { |
||
| 146 | $this->lieuD = trim($this->lieuD); |
||
| 147 | } |
||
| 148 | if (isset($this->lieuA)) { |
||
| 149 | $this->lieuA = trim($this->lieuA); |
||
| 150 | } |
||
| 151 | if (isset($this->heureD)) { |
||
| 152 | $this->heureD = trim($this->heureD) . '00'; |
||
| 153 | } |
||
| 154 | if (isset($this->heureA)) { |
||
| 155 | $this->heureA = trim($this->heureA) . '00'; |
||
| 156 | } |
||
| 157 | if (isset($this->BBC_ballons_idBBC_ballons)) { |
||
| 158 | $this->BBC_ballons_idBBC_ballons = trim($this->BBC_ballons_idBBC_ballons); |
||
| 159 | } |
||
| 160 | if (isset($this->nbrPax)) { |
||
| 161 | $this->nbrPax = trim($this->nbrPax); |
||
| 162 | } |
||
| 163 | if (isset($this->remarque)) { |
||
| 164 | $this->remarque = trim($this->remarque); |
||
| 165 | } |
||
| 166 | if (isset($this->incidents)) { |
||
| 167 | $this->incidents = trim($this->incidents); |
||
| 168 | } |
||
| 169 | if (isset($this->fk_type)) { |
||
| 170 | $this->fk_type = trim($this->fk_type); |
||
| 171 | } |
||
| 172 | if (isset($this->fk_pilot)) { |
||
| 173 | $this->fk_pilot = trim($this->fk_pilot); |
||
| 174 | } |
||
| 175 | if (isset($this->fk_organisateur)) { |
||
| 176 | $this->fk_organisateur = trim($this->fk_organisateur); |
||
| 177 | } |
||
| 178 | if (isset($this->is_facture)) { |
||
| 179 | $this->is_facture = trim($this->is_facture); |
||
| 180 | } |
||
| 181 | if (isset($this->kilometers)) { |
||
| 182 | $this->kilometers = trim($this->kilometers); |
||
| 183 | } |
||
| 184 | if (isset($this->cost)) { |
||
| 185 | $this->cost = trim($this->cost); |
||
| 186 | } |
||
| 187 | if (isset($this->fk_receiver)) { |
||
| 188 | $this->fk_receiver = trim($this->fk_receiver); |
||
| 189 | } |
||
| 190 | if (isset($this->justif_kilometers)) { |
||
| 191 | $this->justif_kilometers = trim($this->justif_kilometers); |
||
| 192 | } |
||
| 193 | |||
| 194 | |||
| 195 | // Check parameters |
||
| 196 | // Put here code to add control on parameters values |
||
| 197 | |||
| 198 | // Insert request |
||
| 199 | $sql = 'INSERT INTO ' . MAIN_DB_PREFIX . $this->table_element . '('; |
||
| 200 | |||
| 201 | $sql .= 'date,'; |
||
| 202 | $sql .= 'lieuD,'; |
||
| 203 | $sql .= 'lieuA,'; |
||
| 204 | $sql .= 'heureD,'; |
||
| 205 | $sql .= 'heureA,'; |
||
| 206 | $sql .= 'BBC_ballons_idBBC_ballons,'; |
||
| 207 | $sql .= 'nbrPax,'; |
||
| 208 | $sql .= 'remarque,'; |
||
| 209 | $sql .= 'incidents,'; |
||
| 210 | $sql .= 'fk_type,'; |
||
| 211 | $sql .= 'fk_pilot,'; |
||
| 212 | $sql .= 'fk_organisateur,'; |
||
| 213 | $sql .= 'is_facture,'; |
||
| 214 | $sql .= 'kilometers,'; |
||
| 215 | $sql .= 'cost,'; |
||
| 216 | $sql .= 'fk_receiver,'; |
||
| 217 | $sql .= 'justif_kilometers,'; |
||
| 218 | $sql .= 'date_creation,'; |
||
| 219 | $sql .= 'date_update'; |
||
| 220 | |||
| 221 | |||
| 222 | $sql .= ') VALUES ('; |
||
| 223 | |||
| 224 | $sql .= ' ' . (!isset($this->date) || dol_strlen($this->date) == 0 ? 'NULL' : "'" . $this->db->idate($this->date) . "'") . ','; |
||
| 225 | $sql .= ' ' . (!isset($this->lieuD) ? 'NULL' : "'" . $this->db->escape($this->lieuD) . "'") . ','; |
||
| 226 | $sql .= ' ' . (!isset($this->lieuA) ? 'NULL' : "'" . $this->db->escape($this->lieuA) . "'") . ','; |
||
| 227 | $sql .= ' ' . (!isset($this->heureD) ? 'NULL' : "'" . $this->heureD . "'") . ','; |
||
| 228 | $sql .= ' ' . (!isset($this->heureA) ? 'NULL' : "'" . $this->heureA . "'") . ','; |
||
| 229 | $sql .= ' ' . (!isset($this->BBC_ballons_idBBC_ballons) ? 'NULL' : $this->BBC_ballons_idBBC_ballons) . ','; |
||
| 230 | $sql .= ' ' . (!isset($this->nbrPax) ? 'NULL' : "'" . $this->db->escape($this->nbrPax) . "'") . ','; |
||
| 231 | $sql .= ' ' . (!isset($this->remarque) ? 'NULL' : "'" . $this->db->escape($this->remarque) . "'") . ','; |
||
| 232 | $sql .= ' ' . (!isset($this->incidents) ? 'NULL' : "'" . $this->db->escape($this->incidents) . "'") . ','; |
||
| 233 | $sql .= ' ' . (!isset($this->fk_type) ? 'NULL' : $this->fk_type) . ','; |
||
| 234 | $sql .= ' ' . (!isset($this->fk_pilot) ? 'NULL' : $this->fk_pilot) . ','; |
||
| 235 | $sql .= ' ' . (!isset($this->fk_organisateur) ? 'NULL' : $this->fk_organisateur) . ','; |
||
| 236 | $sql .= ' ' . (!isset($this->is_facture) ? '0' : $this->is_facture) . ','; |
||
| 237 | $sql .= ' ' . (!isset($this->kilometers) || empty($this->kilometers) ? '0' : $this->kilometers) . ','; |
||
| 238 | $sql .= ' ' . (!isset($this->cost) ? 'NULL' : "'" . $this->db->escape($this->cost) . "'") . ','; |
||
| 239 | $sql .= ' ' . (!isset($this->fk_receiver) ? 'NULL' : $this->fk_receiver) . ','; |
||
| 240 | $sql .= ' ' . (!isset($this->justif_kilometers) ? 'NULL' : "'" . $this->db->escape($this->justif_kilometers) . "'") . ','; |
||
| 241 | $sql .= ' ' . "'" . date('Y-m-d H:i:s') . "'" . ','; |
||
| 242 | $sql .= ' ' . "'" . date('Y-m-d H:i:s') . "'" . ''; |
||
| 243 | |||
| 244 | |||
| 245 | $sql .= ')'; |
||
| 246 | |||
| 247 | $this->db->begin(); |
||
| 248 | |||
| 249 | $resql = $this->db->query($sql); |
||
| 250 | if (!$resql) { |
||
| 251 | $error++; |
||
| 252 | $this->errors[] = 'Error ' . $this->db->lasterror(); |
||
| 253 | dol_syslog(__METHOD__ . ' ' . join(',', $this->errors), LOG_ERR); |
||
| 254 | } |
||
| 255 | |||
| 256 | if (!$error) { |
||
| 257 | $this->id = $this->db->last_insert_id(MAIN_DB_PREFIX . $this->table_element); |
||
| 258 | |||
| 259 | if (!$notrigger) { |
||
|
|
|||
| 260 | // Uncomment this and change MYOBJECT to your own tag if you |
||
| 261 | // want this action to call a trigger. |
||
| 262 | |||
| 263 | //// Call triggers |
||
| 264 | //$result=$this->call_trigger('MYOBJECT_CREATE',$user); |
||
| 265 | //if ($result < 0) $error++; |
||
| 266 | //// End call triggers |
||
| 267 | } |
||
| 268 | } |
||
| 269 | |||
| 270 | // Commit or rollback |
||
| 271 | if ($error) { |
||
| 272 | $this->db->rollback(); |
||
| 273 | |||
| 274 | return -1 * $error; |
||
| 275 | } else { |
||
| 276 | $this->db->commit(); |
||
| 277 | |||
| 278 | return $this->id; |
||
| 279 | } |
||
| 280 | } |
||
| 281 | |||
| 282 | /** |
||
| 283 | * Load object in memory from the database |
||
| 284 | * |
||
| 285 | * @param int $id Id object |
||
| 286 | * @param string $ref Ref |
||
| 287 | * |
||
| 288 | * @return int <0 if KO, 0 if not found, >0 if OK |
||
| 289 | */ |
||
| 290 | public function fetch($id, $ref = null) |
||
| 370 | |||
| 371 | /** |
||
| 372 | * Load object in memory from the database |
||
| 373 | * |
||
| 374 | * @param string $sortorder Sort Order |
||
| 375 | * @param string $sortfield Sort field |
||
| 376 | * @param int $limit offset limit |
||
| 377 | * @param int $offset offset limit |
||
| 378 | * @param array $filter filter array |
||
| 379 | * @param string $filtermode filter mode (AND or OR) |
||
| 380 | * |
||
| 381 | * @return int <0 if KO, >0 if OK |
||
| 382 | */ |
||
| 383 | public function fetchAll( |
||
| 480 | |||
| 481 | /** |
||
| 482 | * Update object into database |
||
| 483 | * |
||
| 484 | * @param User $user User that modifies |
||
| 485 | * @param bool $notrigger false=launch triggers after, true=disable triggers |
||
| 486 | * |
||
| 487 | * @return int <0 if KO, >0 if OK |
||
| 488 | */ |
||
| 489 | public function update(User $user, $notrigger = false) |
||
| 607 | |||
| 608 | /** |
||
| 609 | * Delete object in database |
||
| 610 | * |
||
| 611 | * @param User $user User that deletes |
||
| 612 | * @param bool $notrigger false=launch triggers after, true=disable triggers |
||
| 613 | * |
||
| 614 | * @return int <0 if KO, >0 if OK |
||
| 615 | */ |
||
| 616 | View Code Duplication | public function delete(User $user, $notrigger = false) |
|
| 659 | |||
| 660 | /** |
||
| 661 | * Return a link to the user card (with optionaly the picto) |
||
| 662 | * Use this->id,this->lastname, this->firstname |
||
| 663 | * |
||
| 664 | * @param int $withpicto Include picto in link (0=No picto, 1=Include picto into link, 2=Only picto) |
||
| 665 | * @param string $option On what the link point to |
||
| 666 | * @param integer $notooltip 1=Disable tooltip |
||
| 667 | * @param int $maxlen Max length of visible user name |
||
| 668 | * @param string $morecss Add more css on link |
||
| 669 | * |
||
| 670 | * @return string String with URL |
||
| 671 | */ |
||
| 672 | public function getNomUrl($withpicto = 0, $option = '', $notooltip = 0, $maxlen = 24, $morecss = '') |
||
| 700 | |||
| 701 | /** |
||
| 702 | * Retourne le libelle du status d'un user (actif, inactif) |
||
| 703 | * |
||
| 704 | * @param int $mode 0=libelle long, 1=libelle court, 2=Picto + Libelle court, 3=Picto, 4=Picto + Libelle long, 5=Libelle court + Picto |
||
| 705 | * |
||
| 706 | * @return string Label of status |
||
| 707 | */ |
||
| 708 | public function getLibStatut($mode = 0) |
||
| 712 | |||
| 713 | /** |
||
| 714 | * Renvoi le libelle d'un status donne |
||
| 715 | * |
||
| 716 | * @param int $status Id status |
||
| 717 | * @param int $mode 0=libelle long, 1=libelle court, 2=Picto + Libelle court, 3=Picto, 4=Picto + Libelle long, 5=Libelle court + Picto |
||
| 718 | * |
||
| 719 | * @return string Label of status |
||
| 720 | */ |
||
| 721 | private function LibStatut($status, $mode = 0) |
||
| 779 | |||
| 780 | /** |
||
| 781 | * @return string |
||
| 782 | */ |
||
| 783 | public function __toString() |
||
| 787 | |||
| 788 | /** |
||
| 789 | * @return string |
||
| 790 | */ |
||
| 791 | public function toString() |
||
| 795 | |||
| 796 | /** |
||
| 797 | * @return string |
||
| 798 | */ |
||
| 799 | public function getStatus() |
||
| 803 | |||
| 804 | /** |
||
| 805 | * @return boolean |
||
| 806 | */ |
||
| 807 | public function hasFacture() |
||
| 811 | |||
| 812 | /** |
||
| 813 | * @param int $userId |
||
| 814 | * |
||
| 815 | * @return User |
||
| 816 | */ |
||
| 817 | private function fetchUser($userId) |
||
| 824 | |||
| 825 | /** |
||
| 826 | * @return Bbc_ballons |
||
| 827 | */ |
||
| 828 | private function fetchBalloon() |
||
| 835 | |||
| 836 | /** |
||
| 837 | * @return Bbc_ballons |
||
| 838 | */ |
||
| 839 | public function getBalloon() |
||
| 847 | |||
| 848 | /** |
||
| 849 | * @return User |
||
| 850 | */ |
||
| 851 | public function getPilot() |
||
| 859 | |||
| 860 | /** |
||
| 861 | * @return Bbctypes |
||
| 862 | */ |
||
| 863 | public function getFlightType() |
||
| 870 | |||
| 871 | /** |
||
| 872 | * @return string |
||
| 873 | */ |
||
| 874 | public function getComment() |
||
| 878 | |||
| 879 | /** |
||
| 880 | * @return string |
||
| 881 | */ |
||
| 882 | public function getIncident() |
||
| 886 | |||
| 887 | /** |
||
| 888 | * Return true if the number of pax is greater than 0 |
||
| 889 | * |
||
| 890 | * @return boolean |
||
| 891 | */ |
||
| 892 | public function hasPax() |
||
| 896 | |||
| 897 | /** |
||
| 898 | * Regarding the type of the flight give an indication if the flight must have pax to be valid. |
||
| 899 | * |
||
| 900 | * @return boolean |
||
| 901 | */ |
||
| 902 | public function mustHavePax() |
||
| 906 | |||
| 907 | /** |
||
| 908 | * Returns true if the amount requested by the flight is 0. |
||
| 909 | * |
||
| 910 | * @return boolean |
||
| 911 | */ |
||
| 912 | public function isFree() |
||
| 916 | |||
| 917 | /** |
||
| 918 | * @return int |
||
| 919 | */ |
||
| 920 | public function getAmountReceived() |
||
| 924 | |||
| 925 | /** |
||
| 926 | * @return int |
||
| 927 | */ |
||
| 928 | public function getAmountPerPassenger() |
||
| 932 | |||
| 933 | /** |
||
| 934 | * @return boolean |
||
| 935 | */ |
||
| 936 | public function hasReceiver() |
||
| 940 | |||
| 941 | /** |
||
| 942 | * @return boolean |
||
| 943 | */ |
||
| 944 | public function hasKilometers() |
||
| 948 | |||
| 949 | /** |
||
| 950 | * @return boolean |
||
| 951 | */ |
||
| 952 | public function hasKilometersDescription() |
||
| 956 | |||
| 957 | /** |
||
| 958 | * @return int |
||
| 959 | */ |
||
| 960 | public function getKilometers() |
||
| 964 | |||
| 965 | /** |
||
| 966 | * @return string |
||
| 967 | */ |
||
| 968 | public function getPlaces() |
||
| 972 | } |
||
| 973 | |||
| 1008 |
Loading history...
This check looks for the bodies of
ifstatements 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
ifbodies can be removed. If you have an empty if but statements in theelsebranch, consider inverting the condition.could be turned into
This is much more concise to read.