Opened 9 months ago
Closed 8 months ago
#1854 closed Frage (fixed)
Uneindeutige if-Clause in Methode get_products() in shopping_cart Klasse
Reported by: | noRiddle | Owned by: | somebody |
---|---|---|---|
Priority: | normal | Milestone: | modified-shop-2.0.6.0 |
Component: | Shop | Version: | 2.0.5.1 |
Description
In der Methode get_products() wird am Anfang folgendes abgefragt:
if($this->contents[$products_id]['qty'] != 0 || $this->contents[$products_id]['qty'] !=''){
Ich gehe davon aus, daß man hier "weder 0 noch leer" meint und es sollte deshalb vielleicht besser AND anstatt OR lauten:
if($this->contents[$products_id]['qty'] != 0 && $this->contents[$products_id]['qty'] !=''){
Wenn z.B. 0 als String, also '0' der Wert für qty wäre ergäbe die if-Clause true, nur weil es ein INT ist ergibt sie false.
Test:
$v = 0; var_dump($v != 0 || $v != ''); //false (imho überraschend weil nicht intuitiv)
während
$v = '0'; var_dump($v != 0 || $v != ''); //true
Imho ist eigentlich beides nicht gut weil kein (int)0 und kein '', also leerer String, nicht auf der selben Ebene spielt.
Entweder man stellt vorher sicher, daß qty immer ein INT ist oder immer ein String und fragt entsprechend logisch ab oder man sollte empty() benutzen.
if(!empty($this->contents[$products_id]['qty'])){
Gruß,
noRiddle
Attachments (0)
Change History (2)
comment:1 Changed 9 months ago by noRiddle
comment:2 Changed 8 months ago by GTB
- Resolution set to fixed
- Status changed from new to closed
In 12847:
Ein ähnliches Problem existiert übrigens auch bei den mySQL-Updates für die Shop-Updates, wo mySQL schonmal meldet "Verkehrter Wert für Integer blablabla", weil bei Auto-Increment-Feldern die einen INT haben in den Befehlen oft ein leerer String steht ('') anstatt NULL.
Beispiel aus Update von 1.06.4 zu 2.0.0.0:
Einen leeren String in ein INT-Feld zu laden ist halt nicht korrekt.
Ich könnte mir vorstellen, daß das in zukünftigen mySQL-Versionen gar nicht mehr akzeptiert wird.
Gruß,
noRiddle