Vinzenz Mai: Verzeichnisse auslesen

Beitrag lesen

Hallo Robert,

vorab: meine folgende Kritik ist nicht persönlich gemeint.

Ich habs mal so probiert:

und warum finde ich keinen einzigen Kommentar außer auskommentiertem Code. Unkommentierter Code ist miserabler Code. Dein Code erklärt sich - wie nahezu jeder Code - nicht von selbst.

  
// Was ist die Aufgabe dieser Funktion?  
// Kommentar: sie hat einen verbesserungswürdigen Namen  
// Welche Parameter nimmt sie entgegen?  
// Kommentar: der Name des Parameters ist ok.  
// Was gibt sie zurück?  
// Warum gibt sie nichts zurück?  
// Warum gibt sie etwas aus?  
// Warum gibt sie HTML aus?  
// Wenn sie schon HTML ausgibt, warum ist die Ausgabe nicht kontextgemäß aufbereitet?  
  
// Zum Coding-Style: 1 Zeichen Einrückung ist eine Zumutung!  

> function overview ($dir) {  
> //  echo "<h3>dir: $dir</h3>\n";  
  
//  Was durchläufst Du mit dieser Schleife  
//  $dir_or_file ist ein verbesserungswürdiger Variablenname  

>   foreach(scandir($dir) as $dir_or_file) {  
  
//  Was bezweckt diese Prüfung? Das ist nicht offensichtlich!  

>   if (strpos($dir_or_file,".")!==0) {  
  
//   Was ist der Sinn dieser Prüfung?  

>    if (is_dir($dir.'/'.$dir_or_file))  {  
  
//   Warum wird hier etwas ausgegeben?  

>     echo "<li>$dir_or_file<ul>\n";  
  
//   Warum erfolgt der rekursive Aufruf? Begründe es!  
//   Bist Du Dir sicher, dass Du in jedem Fall absteigen musst?  

>     overview($dir.'/'.$dir_or_file);  
  

>     echo "</ul></li>\n";  
>    }  
>    else {  

// Aha, welchen Fall haben wir hier. Schreibe es hin!  
  
// Was soll das? Begründe!  

>     if (true) {  
  
//strpos($dir_or_file,".php" ) || strpos( $dir_or_file, ".htm")) {  
  
/*  
   Kannst Du Dir vorstellen, dass es Zeichen in Dateinamen geben kann,  
   die im HTML-Kontext behandelt werden müssen. Warum machst  
   Du dies nicht?  
   Welche Spezialbedingung gilt für den Inhalt von $dir?  
  
   Anmerkung zum Coding-Style:  
   Ich halte es für eine gute Idee, Operatoren durch Leerzeichen von den  
   Operanden zu trennen. Es fällt mir schwer, Deinen Code zu lesen.  
  
   Viel sinnvoller wäre es selbstverständlich, wenn diese Funktion nichts  
   ausgäbe, stattdessen etwas zurückgäbe.  
*/  

>      echo '<li><a href="' .$dir. "/" .$dir_or_file. '">'. $dir_or_file. '</a></li>'."\n";  
>     } else  
>     echo "<li>$dir_or_file</li>\n";  
>    }  
>   }  
>  }  
> }  
> overview(".");  
> 

Fazit:
Eine höchst spezifische Funktion, überhaupt nicht dokumentiert - und wie Tom anmerkte dazu anfällig für Endlosschleifen.

Meine ernstgemeinte Bitte:

Wenn man schon Code wie diesen, eine einfache Funktion vorstellt, dann sollte diese *ausreichend* und ordentlich dokumentiert sein. Dies bedeutet, dass die Aufgabe beschrieben ist und man *nicht* den Code durchackern muss, um sie zu verstehen.

Code wie der vorgestellte ist nicht hilfreich!
Obwohl der Fragesteller mit Deinem Code zufrieden war, bin ich der Ansicht, dass Deine Funktion nur als Beispiel dafür dient, wie man es *nicht* machen sollte.

Konstruktive Grüße

Vinzenz