Listing-Filter: Performance und unsinnige Query

[noRiddle]

In der Template-Datei zum Listing-Filter (/templates/tpl_modified/module/listing_filter.html) wird mittels folgendem Code

{if count($FILTER_TAG) < 1 && $FILTER_MANUFACTURER != ''}<div class="sort_bar_item">{$FILTER_MANUFACTURER}</div>{/if}

conditional das Hersteller-Dropdown angezeigt oder eben nicht.
Der Code zu dem $FILTER_MANUFACTURER in der /includes/modules/listing_filter.php wird jedoch immer ausgeführt.
Das halte ich für ein Performance-Manko. Man sollte die Condition in den PHP-Code einbauen, sodaß der Code erst gar nicht ausgeführt wird wenn

count($filter_dropdown) < 1

Der Sinn dieser Condition entzieht sich allerdings ohnehin meinem Verständnis.
Lediglich wenn man products_tags bzgl. Hersteller angelegt hätte würde ich die Condition verstehen.
Aber das ist ein Nebenthema.

Es gibt im Code des Listing-Filters allerdings noch ein anderes Problem.
Wir haben diesen Code:

  $filter_join = '';
  if (isset($_GET['filter']) && is_array($_GET['filter'])) {
    $fi = 1;
    foreach ($_GET['filter'] as $options_id => $values_id) {
      if ($values_id != '') {
        $filter_join .= "JOIN ".TABLE_PRODUCTS_TAGS." pt".$fi."
                              ON pt".$fi.".products_id = p.products_id
                                 AND pt".$fi.".options_id = '".(int)$options_id."'
                                 AND pt".$fi.".values_id = '".(int)$values_id."' ";
        $fi ++;
      }
    }
  }

Die Nutzung von $filter_join in der Query die Manufacturers betreffend ist nachvollziehbar.
Was allerdings die Query für die Filter betrifft ist die Verwendung nicht nachvollziehbar.
Das hat zwei Gründe:

1. gibt es damit zwei JOINs auf TABLE_PRODUCTS_TAGS, was, wenn man meint das so haben zu wollen, man so umschreiben sollte, daß es den JOIN nur einmal gibt, eben je nach Fall

        ON pt.products_id = p.products_id
   

oder eben

     ON pt".$fi.".products_id = p.products_id
    AND pt".$fi.".options_id = '".(int)$options_id."'
    AND pt".$fi.".values_id = '".(int)$values_id."' "

aber nicht beides.

2. stellt sich die Frage warum das Zweitgenannte überhaupt gemacht wird.

Es ist für den Kunden verwirrend wenn er einen Wert aus einem Filter-Dropdown ausgewählt hat und dann in diesem Dropdown nur noch den ausgewählten Wert sieht,
zusätztlich zu "NAME (alle anzeigen)" versteht sich,
anstatt nach wie vor alle Optionen inkl. des "NAME (alle anzeigen)".
Wenn man nämlich seine Wahl verändern möchte benötigt es zwei Klicks anstatt eines einzigen.
Das habe ich so wie es jetzt ist auch noch nirgends gesehen.

Gibt es einen Grund das so zu machen den ich nicht kenne ?

Gruß,
noRiddle

[GTB]

{if count($FILTER_TAG) < 1 && $FILTER_MANUFACTURER != ''}<div class="sort_bar_item">{$FILTER_MANUFACTURER}</div>{/if}

Dieser Code hat nur mit der Darstellung zu tun. Um herauszufinden ob es Hersteller gibt, muss ein SQL gemacht werden, sofern die Filter aktiv sind. Das Gleiche gilt für die Eigenschaften.

  $filter_join = '';
  if (isset($_GET['filter']) && is_array($_GET['filter'])) {
    $fi = 1;
    foreach ($_GET['filter'] as $options_id => $values_id) {
      if ($values_id != '') {
        $filter_join .= "JOIN ".TABLE_PRODUCTS_TAGS." pt".$fi."
                              ON pt".$fi.".products_id = p.products_id
                                 AND pt".$fi.".options_id = '".(int)$options_id."'
                                 AND pt".$fi.".values_id = '".(int)$values_id."' ";
        $fi ++;
      }
    }
  }

Dieser Codeblock ist notwendig, damit nur mehr die Hersteller und Eigenschaften angezeigt werden, welche die aktuelle Auswahl haben. Zudem stellt es sicher, dass es keine leeren Ergebnisse gibt. Deshalb werden auch Optionen ausgeblendet bei einer Auswahl.

[noRiddle]

Das Problem auf welches ich aufmerksam machen möchte hat die Ursache, daß man Code zusammengefasst hat den man nicht zusammenfassen sollte.
$filter_join wird sowohl in der Query für die Manufacturers als auch in der Query für die Filter benutzt.
In der Query für die Filter kommt es dadurch zu einem doppelten JOIN auf TABLE_PRODUCTS_TAGS was die Query unnötig verkompliziert.
Viel wichtiger aber ist das was ich eingangs unter Punkt 2. geschrieben habe. Warum zwingt man den User für den Wunsch eine Filteroption zu ändern zweimal zu klicken, einmal um "alle anzeigen" zu lassen und das zweite mal um dann den neuen Filter zu wählen.
Der JOIN in der Query für die Filter

JOIN ".TABLE_PRODUCTS_TAGS." pt
     ON pt.products_id = p.products_id

ist doch entscheidend und völlig ausreichend.
Ich sehe keinen Grund das zweite JOIN aus der Variablen $filter_join zu machen.
Deshalb schrieb ich ja auch
"Die Nutzung von $filter_join in der Query die Manufacturers betreffend ist nachvollziehbar.
Was allerdings die Query für die Filter betrifft ist die Verwendung nicht nachvollziehbar."

Dein Satz, GTB,
"Dieser Codeblock ist notwendig, damit nur mehr die Hersteller und Eigenschaften angezeigt werden, welche die aktuelle Auswahl haben."
trifft lediglich auf die Query für die Manufacturers zu.

Deine Aussage zur Vermeidung von leeren Ergebnissen kann ich nicht nachvollziehen, habe ich aber auch zugegebenermaßen nicht zu Ende durchdacht.

Falls mir bzgl. $filter_join der Durchblick fehlen sollte bitte ich um Nachsicht.

---

Die andere Sache mit dem Auslagern der Condition in den PHP-Code habe ich verkehrt eingeschätzt, gebe dir Recht.
Im PHP-Code wird ja nach der notwedigen Query der Rest des Codes bereits unter der Condition

if (xtc_db_num_rows($filterlist_query, true) > 0) {

ausgeführt. Also alles gut.

[noRiddle]

Schaust du dir meine Argumentation im letzten Post bitte nochmals an, GTB ?
Ich bin mir ziemlich sicher, daß meine Ausführungen begründet sind.

Gruß,
noRiddle

[GTB]

der erste JOIN ist dass nur die verfügbaren Eigenscahften angezeigt werden, alle weiteren schränken es auf die ausgewählten Eigenschaften ein, damit es keine leeren Ergebnisse gibt.

[noRiddle]

Ich verstehe es immer noch nicht.
Im $listing_sql der /includes/modules/default.php wird doch nach $_GET[filter] für die Anzeige der Produkte gefiltert und das JOIN in der Variablen $from ist exakt dasselbe wie in der Variablen $filter_join in /includes/modules/listing_filter.php.
Wie sollte es leere Ergebnisse bringen wenn man bei der Query $filterlist_sql die dafür da ist die Dropdowns zu bilden auf (die imho überflüssige) $filter_join verzichtet ? Leere Ergebnisse von was genau ?

Übrigens wäre es überlegenswert keinen Filter anzeigen zu lassen der weniger als 2 Werte hat:

      //BOC added condition to show only filters with more than one option, noRiddle
      //$options_array = array_merge($options_array, $values);
      if(count($values) > 1 || (isset($_GET['filter'][$options_id]) && $_GET['filter'][$options_id] != '')) {
        $options_array = array_merge($options_array, $values);
      } else {
        unset($options_array);
      }
      //EOC added condition to show only filters with more than one option, noRiddle
       if(!empty($options_array)) { //added condition to show only filters with more than one option, noRiddle
          $filter_dropdown[$options_id] = xtc_draw_form('filter_'.$options_id, xtc_href_link(basename($PHP_SELF), xtc_get_all_get_params(array('page', 'show', 'cat'))), 'get');
          if (isset($_GET['manufacturers_id']) && $_GET['manufacturers_id'] > 0) {
            if (basename($PHP_SELF) == FILENAME_ADVANCED_SEARCH_RESULT || SEARCH_ENGINE_FRIENDLY_URLS != 'true') {
              $filter_dropdown[$options_id] .= xtc_draw_hidden_field('manufacturers_id', (int)$_GET['manufacturers_id']).PHP_EOL;
            }
          }
          ...
      } //END added condition to show only filters with more than one option, noRiddle

Gruß,
noRiddle

[Tomcraft]

Ruf doch einfach mal Gerhard an. Das wird die Sache sicherlich beschleunigen. ;-)