OOCategory::getParent.. sinnlose query @ root

[jdlx]

Moin,

mir ist heute aufgefallen, daß getParent() nicht prüft, ob es dabei ist, die root category 0 aufzurufen.. , denn dies führt letztlich via OOArticle::getArticleById zur sinnfreien sql query

Code: Alles auswählen

SELECT * FROM rex_article WHERE rex_article.id='0' AND clang='0'

.. und das jedesmal bei einem Seitenaufruf. Wenn jetzt irgendein code sich rekursiv durch die Kategorien hangelt, gibts für jedesmal "in root aufschlagen" diese query. Wenn dann pro Seite (hier in einem Rexsale Projekt) mal eben 22 dieser queries mit aufn Zettel kommen, ist das schon ne Spaßbremse.

Ich hab hier jetzt erstmal die Funktion wie folgt geändert:

Code: Alles auswählen

 // Original
/*public*/ function getParent($clang = false)
{
  if ($clang === false)
    $clang = $this->_clang;
  return OOCategory :: getCategoryById($this->_re_id, $clang);
}

Code: Alles auswählen

 // Patch
/*public*/ function getParent($clang = false)
{
  if ($clang === false)
    $clang = $this->_clang;
  if($this->_re_id>0)
  {
    return OOCategory :: getCategoryById($this->_re_id, $clang);
  }
  else
  {
    return null;
  }
}

Meinungen?

lg,
Jan

[Xong]

Meinungen?

Man könnte hier argumentieren, dass der Programmierer selbst dafür verantwortlich ist und überprüfen muss, ob der Einsatz von getParent() notwendig ist.

Mir fällt ehrlich gesagt auch keine Situation ein, in der ein rekursiver Aufruf von getParent() nötig ist. Entweder man hangelt sich von oben nach unten durch oder man arbeitet mit dem Pfad (getParentTree()) des jeweiligen Artikels bzw. der Kategorie.

[jdlx]

Man könnte hier argumentieren, dass der Programmierer selbst dafür verantwortlich ist und überprüfen muss, ob der Einsatz von getParent() notwendig ist.

Seh ich persönlich anders.. u.a. auch wegen dem unnötigen Aufwand das im eigenen Code zu kontrollieren. Eine sinnfreie live query, die lediglich durch Verwendung einer core Funktion auftritt is imho eher ein bug.

Mir fällt ehrlich gesagt auch keine Situation ein, in der ein rekursiver Aufruf von getParent() nötig ist.

Ob es sinnvoll erscheinen mag oder nicht finde ich an der Stelle müßig..

lg,
Jan

[xrstf]

Ich find die Lösung von Jan praktikabel. Das CMS hat an der Stelle bereits genug Wissen über die Struktur und kann problemlos die Abfrage einspraren. REDAXO ist ja kein nacktes Framework à la Doctrine, das nichts über die Semantik der Daten weiß.

Von daher: +1

[Markus.Staab]

Hi zusammen,

Ich finde auch, dass es kein dringender fix ist, allerdings trivial und sinnvoll.

Daher werden wir das in der R5 beheben,...

Lg, Markus

[Markus.Staab]

Hi nochmal,

habs mir grad nochmal angesehen... in R5 tritt das Problem gar nicht auf.

Gruß,
Markus

[Ruediger.Nitzsche]

ein Fix für R4 wäre aber trotzdem sinnvoll, da ja zum einen R5 noch nicht veröffentlicht ist und die 4 garantiert noch eine ganze Weile genutzt werden wird als Produktivsystem.

Rüdiger

[Hirbod]

+1 @ Rüdiger.

Ich finde es auch schade, das hier ständig nur Bugfixes für R5 nachgeschoben werden. Im Forum sind mittlerweile genug Bugs für R4.3.* aufgetaucht, nen 4.3.3 mit paar Bugfixes nachzuschieben finde ich sinnvoll.

[Markus.Staab]

Hi,

das ist kein Bug sondern ein min. Performance Verbesserung in einem Edge-Case.

Gruß,
Markus

[jdlx]

das ist kein Bug sondern ein min. Performance Verbesserung in einem Edge-Case.

Wie auch immer man das persönlich bewerten will, was spricht dagegen es zu fixen?
Zumal der fix trivial und ohne Nebenwirkungen ist..

rex5 in allen Ehren, aber um das Mantra der anderen nochmal zu wiederholen: rex4 ist nunmal die aktuelle Version.. es wird nicht jeder gleich seine site auf r5 updaten wollen oder können wenn es dann mal als stable zur Verfügung steht, genauso wie aktuell viele Sites noch mir 4.x laufen..
d.h. es spricht alles dafür aufgestaute fixes für r4 auch tatsächlich mal einzubauen.

lg,
Jan