Seite 1 von 2
Bug in Contenido_FrontendNavigation::isActiveChild
Verfasst: Di 28. Apr 2009, 13:53
von sier
Hallo
musste etwas an der Top-Navigation herumbasteln. Dabei habe ich nachfolgenden Bug entdeckt. Wie und wo werden eigentlich Bug gemeldet??
Code: Alles auswählen
public function isActiveChild(Contenido_Category $oCategory, $iCurrentIdcat) {
if ($oCategory->getSubCategories()->count() > 0) {
$iCurrentIdcat = (int) $iCurrentIdcat;
$oChildCategories = $oCategory->getSubCategories();
foreach ($oChildCategories as $oChildCat) {
if ($oChildCat->getIdCat() == $iCurrentIdcat) {
return true;
}
$this->isActiveChild($oChildCat, $iCurrentIdcat)
}
return false;
}
return false;
}
Richtig wäre aber :
Code: Alles auswählen
public function isActiveChild(Contenido_Category $oCategory, $iCurrentIdcat) {
if ($oCategory->getSubCategories()->count() > 0) {
$iCurrentIdcat = (int) $iCurrentIdcat;
$oChildCategories = $oCategory->getSubCategories();
foreach ($oChildCategories as $oChildCat) {
if ($oChildCat->getIdCat() == $iCurrentIdcat) {
return true;
}
if ($this->isActiveChild($oChildCat, $iCurrentIdcat)) {
return true;
}
}
return false;
}
return false;
}
Re: Bug in Contenido_FrontendNavigation::isActiveChild
Verfasst: Di 28. Apr 2009, 13:55
von idea-tec
Re: Bug in Contenido_FrontendNavigation::isActiveChild
Verfasst: Di 28. Apr 2009, 14:25
von _wiewo_
Ähm bin ich gerade nur blöd oder blind?
Der originalcode is doch 100% richtig oder? oO
is doch alles genau wie es soll
wozu nen if nochmal rumpacken wenns doch eh nochmal neu durchläuft,
nene, kein bug =)
Re: Bug in Contenido_FrontendNavigation::isActiveChild
Verfasst: Di 28. Apr 2009, 14:38
von Oldperl
Kein Bug
Die Funktion wird rekursiv aufgerufen bis die Bedingung passt oder keine weiteren Elemente zu prüfen sind. Die Rückgaben sollten dementsprechend korrekt sein.
Gruß aus Franken
Ortwin
Re: Bug in Contenido_FrontendNavigation::isActiveChild
Verfasst: Di 28. Apr 2009, 16:33
von sier
welche rückgabe? bei rekursiven aufruf wird die rückgabe ja gar nie abgefragt/berücksichtigt.
Code: Alles auswählen
foreach ($oChildCategories as $oChildCat) {
if ($oChildCat->getIdCat() == $iCurrentIdcat) {
return true;
}
$this->isActiveChild($oChildCat, $iCurrentIdcat) --> WENN hier TRUE zurückkommt muss der foreach abgebrochen werden
}
Re: Bug in Contenido_FrontendNavigation::isActiveChild
Verfasst: Di 28. Apr 2009, 16:41
von _wiewo_
Code: Alles auswählen
foreach ($oChildCategories as $oChildCat){
if ($oChildCat->getIdCat() == $iCurrentIdcat) {
return true;
}
$this->isActiveChild($oChildCat, $iCurrentIdcat);
}
if, also "wenns" so ist, dann returnd er true und fertig is die kiste
nachem return is aus die maus
wenn nich true dann rufter die funktion nochmal auf
isses wieder nich true rufterse nochmal auf
usw,
daher ist das alles schon so richtig wie es da ist
man könnt auch schreiben
Code: Alles auswählen
foreach ($oChildCategories as $oChildCat) {
if ($oChildCat->getIdCat() == $iCurrentIdcat) {
return true;
} else
{
$this->isActiveChild($oChildCat, $iCurrentIdcat);
}
}
was aber aufs gleiche hinausläuft
Re: Bug in Contenido_FrontendNavigation::isActiveChild
Verfasst: Di 28. Apr 2009, 17:02
von sier
nein der code funktioniert nicht.
glaubs mir, mein code macht völlig etwas anders.
in deinem code wird die suche nicht abgebrochen wenn das durchsuchen der unterkategorien erfolgreich war.
dein code funktioniert nur auf dem einem level
gruss
rocco
Re: Bug in Contenido_FrontendNavigation::isActiveChild
Verfasst: Di 28. Apr 2009, 17:12
von idea-tec
wäre es möglich, dass hier verschiedene Gedankengänge über die Arbeitsweise der Funktion existieren?
Re: Bug in Contenido_FrontendNavigation::isActiveChild
Verfasst: Di 28. Apr 2009, 17:16
von Oldperl
nach ein bisserl Nachdenken im Kämmerchen glaube ich das rocco Recht hat. Es fehlt zwar keine if-Bedingung, bzw. diese ist hier nicht nötig, denn es reicht einfach das Ergebnis des rekursiven Aufrufes zu returnen, da hierbei auf alle Fälle entweder False oder True zurückkommt.
Code: Alles auswählen
foreach ($oChildCategories as $oChildCat){
if ($oChildCat->getIdCat() == $iCurrentIdcat) {
return true;
}
return $this->isActiveChild($oChildCat, $iCurrentIdcat);
}
Bevor ich das aber nach Bugs verschiebe werde ich mir das in einer 4.8.12er mal genauer anschauen.
Gruß aus Franken
Ortwin
Re: Bug in Contenido_FrontendNavigation::isActiveChild
Verfasst: Di 28. Apr 2009, 17:18
von Oldperl
idea-tec hat geschrieben:wäre es möglich, dass hier verschiedene Gedankengänge über die Arbeitsweise der Funktion existieren?
@Karsten
ich denke nicht, bei mir wars einfach beim Überfliegen mal wieder zu schnell gewesen
Gruß aus Franken
Ortwin
Re: Bug in Contenido_FrontendNavigation::isActiveChild
Verfasst: Di 28. Apr 2009, 17:27
von sier
idea-tec hat geschrieben:wäre es möglich, dass hier verschiedene Gedankengänge über die Arbeitsweise der Funktion existieren?
schon möglich, hier die funktionsbeschreibung: "Check if current idcat is an active child category of a given Contenido_Category"
aber der aufruf
innerhalb der foreach-scheife im orignal code ist ein bug, unabhängig wie der arbeitsweise gemeint ist.
gruss
rocco
Re: Bug in Contenido_FrontendNavigation::isActiveChild
Verfasst: Di 28. Apr 2009, 17:34
von idea-tec
wenn es am ende so etwas wie ein bug war/ist, und er wird beseitigt, und wir reduzieren dadurch das feuer auf die db -> SUPER!!!
@Oldperl:
ich hatte mir das "nachsehen" auch notiert (aus reiner neugier), stecke aber zu tief in den letzten Zügen meines projektes und komme vor mitte/ende nächster woche sicher nicht dazu.
von daher, streiche ich mir das wieder, da ich das "genauer anschauen" nun in guten Augen weiß.
Re: Bug in Contenido_FrontendNavigation::isActiveChild
Verfasst: Mi 29. Apr 2009, 08:05
von _wiewo_
von der PHP Seite... (
http://php.net/manual/de/functions.user-defined.php)
Beispiel #4 Rekursive Funktionen
Code: Alles auswählen
<?php
function recursion($a)
{
if ($a < 20) {
echo "$a\n";
recursion($a + 1);
}
}
?>
oder
http://pear.php.net/manual/de/standars.errors.php
Code: Alles auswählen
<?php
/**
* Rekursive Suche in einem Baum nach einem String
* @throws ResultException
*/
public function search(TreeNode $node, $data)
{
if ($node->data === $data) {
throw new ResultException( $node );
} else {
search( $node->leftChild, $data );
search( $node->rightChild, $data );
}
}
?>
Also die Funktion einfach so wieder aufrufen ist vom PHP Sinn der Rekursion
Das sind zwar keine Beispiele mit return, aber...
Ein return mittendrin einzubauen wäre meines Erachtens kein sauberer CodingStyle.
Ich würde niemals direkt vor einen Funktionsaufruf ein Return setzen, das gehört sich nich. meines Erachtes, Für andere Meinungen bin ich offen
Finde das ein Interessantes Thema
Re: Bug in Contenido_FrontendNavigation::isActiveChild
Verfasst: Mi 29. Apr 2009, 08:10
von idea-tec
ich bin ja nun selfmade-php und denke, dass das mit dem return schon geht.
ich würde dann allerdings auf jeden fall direkt hinter das return ein exit setzen, weil ich ja mein "ergebnis" habe und auf keinen fall will, dass das ding weiter da durchrauscht.
vor allem könnte es ja auch sein, dass ich das ergebnis nach der 4. von 120 schleifen habe und dann rauscht das noch weitere 116 schleifen da durch, ohne sinn und zweck
Re: Bug in Contenido_FrontendNavigation::isActiveChild
Verfasst: Mi 29. Apr 2009, 08:31
von _wiewo_
nach nem return bricht er eigentlich sofort ab
return
Wird die return() Anweisung innerhalb einer Funktion aufgerufen, wird die Ausführung der Funktion sofort beendet und das Argument als Wert des Funktionsaufrufs zurückgegeben.
http://www.php-resource.de/handbuch/function.return.htm