Opened 3 weeks ago

Closed 43 hours 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.5.2
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 3 weeks ago by 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

comment:2 Changed 43 hours ago by GTB

  • Resolution set to fixed
  • Status changed from new to closed

In 12847:

fix #1854 - fix if clause

Add Comment

Modify Ticket

Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.