Aufbau Methode xtcFormat() in xtcPrice-Class nicht korrekt und nicht effektiv

[anonymous]

In der Methiode xtcFormat() in der Class xtcPrice wird folgende Query gemacht wenn der Parameter $format true ist:

if ($format) {
      $sQuery = xtDBquery("SELECT max(po.quantity) AS qty,
                                  p.products_tax_class_id
                             FROM " . TABLE_PERSONAL_OFFERS_BY . $this->actualGroup . " po
                             JOIN " . TABLE_PRODUCTS . " p
                                  ON po.products_id = p.products_id
                            WHERE po.products_id='" . $pID . "'
                         GROUP BY p.products_id");
      $sQuery = xtc_db_fetch_array($sQuery, true);
...

Diese Query sollte man einschränken mittels

if($pID != 0) {
    ...
}

weil ansonsten bei jeder Verwendung von xtcFormat() mit $format auf true die Query ausgeführt wird, auch wenn $pid == 0 ist.

Ich vermute man hat versucht die Funktion so allgemein zu halten, daß sie in vielen Fällen bennutzt werden kann.
Als Beispiel:
Im Falle der Verwendung der Funktion in /includes/modules/order_details_cart.php macht es überhaupt keinen Sinn die Query auszuführen.

Im imho ohnehin etwas verwirrend aufgebauten Code der genannten Funktion sollte die von mir genannte if-Clause den folgenden Code so umfassen:

if($pID != 0) {
    $sQuery = xtDBquery("SELECT max(po.quantity) AS qty,
                                p.products_tax_class_id
                           FROM " . TABLE_PERSONAL_OFFERS_BY . $this->actualGroup . " po
                           JOIN " . TABLE_PRODUCTS . " p
                             ON po.products_id = p.products_id
                          WHERE po.products_id='" . $pID . "'
                       GROUP BY p.products_id");
    $sQuery = xtc_db_fetch_array($sQuery, true);
    if(($this->cStatus['customers_status_graduated_prices'] == '1') && ($sQuery['qty'] > 1)) {
        $from = ' ' . FROM . ' ';
        $price = $this->xtcGetGraduatedPrice($pID, $sQuery['qty']);
        if($curr) {
            $price = $this->xtcCalculateCurr($price);
        }
        if($sQuery['products_tax_class_id'] != 0) {
            $products_tax = ($this->cStatus['customers_status_show_price_tax'] == '0') ? '' : $this->TAX[$sQuery['products_tax_class_id']];
            $price = $this->xtcAddTax($price, $products_tax);
        }
    }
}

Also bis ausschließlich

$Pprice = $this->xtcFormatCurrency($price, $decimal_places);

---

So wie es bislang ist, ohne die if-Clause, ist ohnehin die Frage ob das

if($sQuery['products_tax_class_id'] != 0) {

nicht eigentlich zu ungenau und logisch unkorrekt ist.
Denn die Query ergibt ja ein leeres Ergebnis wenn $pid == 0 und so wie es jetzt ist müsste dort korrekter das stehen:

if($sQuery['products_tax_class_id'] > 0) {

oder besser gar vor Ausführung von

$sQuery = xtc_db_fetch_array($sQuery, true);

und dem darauf folgedem Code

if(xtc_db:num_rows())

abfragen.

Das o.g. Einfassen des Codes mit if($pID != 0) { erscheint mir allerdings die beste Lösung zu sein.

Gruß,
noRiddle