Nested Sets Klasse

[Opendix]

So jetzt bin ich auch mal wieder hier :)

Ich hab mir das ganze auch mal angeschaut (nur angeschaut, muss erst wieder meinen Server hier installieren bis dann kann ich noch nicht testen :P) und mir sind da der eine oder andere Punkt aufgefallen :)

1. Ich finde, dass die Klasse die Mysql-Verbindung nicht selbst herstellen sollte! Die Klasse wird wahrscheinlich in anderen Projekten eingesetzt bei denen schon vor dem Aufruf der Klasse eine Verbindung mit der Datenbank geöffnet wurde! Zwei mal eine Verbindung öffnen macht von mir aus gesehen keinen Sinn :)

2. Warum wird der Tabellenname dem Konstruktor übergeben und die anderen Mysql-Parameter sind fix in der Klasse gesetzt? :S

3. Warum sind _getNaturalNumber und _getFieldValues "protected"?

4. Da sind noch Debug-Ausgaben zu finden :P

PHP-Code:

echo "LFT: $lft, RGT: $rgt, ROOT: $root"; 


5. Alle Parameter der Funktionen beschreiben! (Aber ich denke da bist du laufend immer ein bisschen dran *g*)

6. Benutzen von Strings optimieren

PHP-Code:

//So machst du es jetzt
"SELECT `lft`, `rgt` FROM `{$this->_dbTblName}` WHERE `id` = {$moveTo}"

//Besser wäre:
'SELECT `lft`, `rgt` FROM `{$this->_dbTblName}` WHERE `id` = \''. $moveTo .'\'' 


7. Ifs ausschreiben

PHP-Code:

//Finde ich persönlich unübersichtlicher und verwirrender als ein 
//normales, ausgeschriebenes IF-Statement! 
//Aber das ist meine Meinung,ich denke andere würden genau das umgekehrte sagen :P
$diff = $hasNoChilds === true ? 2 : ($rgt - $lft) + 1; 


8. Noch ein Punkt zur Übersichtlichkeit

PHP-Code:

//Ich habe in der Schule gelernt, solche Sachen zu vermeiden!
if(!$query = mysql_query($query))
{
   //...
}

//Besser wäre sowas:
$query = mysql_query($query);
if(!$query)
{
    //....
}

//Natürlich ist das wieder Ansichtssache :) 


9. Kein "die" verwenden
Jeder der so eine Klasse verwendet möchte sicherlich auch selbst die Fehler behandeln! Da ist es nicht sehr hilfreich wenn du mit einem die() das Script einfach abbrichst!

PHP-Code:

//Das hier... 
$query = mysql_query("SELECT `lft`, `rgt` FROM `{$this->_dbTblName}` WHERE `id` = {$moveTo}") or die(mysql_error());
//.. wäre so besser zu lösen:
$query = mysql_query('SELECT ....');
if(!$query)
{
    throw new Exception(mysql_error());
} 


10. mysql_fetch_array($query, MYSQL_ASSOC)
Anstatt dieser fetch_array doch einfach mysql_fetch_assoc verwenden :)

11. Noch was zur Übersichtlichkeit

PHP-Code:

//Warum benutzt du so häufig list? Hat das irgendeinen Vorteil? (Ich weis es nicht, benutze diese Funktion eigentlich nie :S)
list($newLft, $newRgt) = array_values(mysql_fetch_array($query, MYSQL_ASSOC));

//Aber in diesem Falle fände ich es einfacher und schöner sowas zu machen:
$daten = mysql_fetch_assoc($query);
$newLft = $daten['lft'];
$newRgt = $daten['rgt']; 


12. SQL Statements innerhalb mysql_query()
Manchmal machst du eine Variabel $sql, speicherst da die Abfrage und führts sie dann so aus:
mysql_query($sql);

Manchmal aber schreibst du einfach das SQL direkt in das mysql_query(...)!
Zweiteres finde ich keine gute Lösung! Solltest du dich dennoch für diese Entscheiden, dann würde ich es einheitlich machen.

13. ...

PHP-Code:

/***
* FtpConnect class allows easy connecting to a ftp-server.
*  
* @package FtpConnect
* @version 1.0
* @tablesize 4
* @author MaurÃ*cio Hanika <tickduck@gmx.de>
* @author Andreas Wilhelm <Andreas2209@web.de>
* @copyright Andreas Wilhelm
**/ 


Na ob das aktuell ist? *g*

14. schliessende PHP-Tags lasse ich immer weg in Dateien in denen sich nur eine Klasse udn sonst nichts befindet! Grund: es werden dann sicherlich keine Leerzeichen die sich nach dem ?> befinden an den Client gesendet! Klar siehst du diese Leerzeichen nicht beim Client, aber solltest du vielleicht nachher irgendwann mal die funktion header verwenden wirst du lange nach dem besagten Zeichen finden was die coole Meldung "Headers already sent... " auslöst :) Ist da jemand anderer Meinung? Hab das mal irgendwo mitgenommen als ich irgendein (leider vergessen welches) Projekt angeschaut habe...




Soo.. einige Punkte sind wichtiger, einige dann wieder überhaupt nicht so :) hoffe ich konnte dir damit ein bisschen weiterhelfen :)

Btw. finde ich es super, dass du die Klasse öffentlich zugänglich machst.. :)