Uneindeutige if-Clause in Methode get_products() in shopping_cart Klasse

[noRiddle]

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

[noRiddle]

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:

INSERT INTO configuration (configuration_id,  configuration_key, configuration_value, configuration_group_id, sort_order, last_modified, date_added, use_function, set_function) VALUES   ('', 'SHIPPING_STATUS_INFOS', '', 17, 14, NULL, NOW(), NULL, 'xtc_cfg_select_content(\'SHIPPING_STATUS_INFOS\',');

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

[Gerhard Waldemair]

In 12847:

fix #1854 - fix if clause