FTP-Client fertig - eure Meinung

Hmm, gut, ich hab mir jetzt nicht jede Zeile angeschaut, aber einiges ist mir doch aufgefallen...

  • Für ein Skript das andere Leute nutzen sollen, ist zu wenig dokumentiert
  • Es ist unübersichtlich
  • cURL ist nicht bei jedem installiert, also solltest du vielleicht Voraussetzungen notieren (zumindest damit alle Funktionen benutzt werden können?)

PHP:
echo "</td></tr>";
      echo "<tr><td>&nbsp;</td></tr>";
      echo "<tr><td><table class=\"neutral\" border=\"0\" cellpadding=\"0\" cellspacing=\"10\">";
      echo "<tr><td>Server:<br>";
      echo "<input type=\"text\" name=\"server2\" size=\"25\"></td></tr>";
      echo "<tr><td>Benutzer:<br>";
      echo "<input type=\"text\" name=\"benutzer2\" size=\"25\"></td></tr>";
      echo "<tr><td>Passwort:<br>";
      echo "<input type=\"password\" name=\"passwort2\" size=\"25\"></td></tr>";
      echo "<tr><td>Zielverzeichnis:<br>";
      echo "<input type=\"text\" name=\"transziel\" size=\"25\" value=\"$ds\"></td></tr>";
      echo "<tr><td>&nbsp;</td></tr>";
      echo "<tr><td><input type=\"submit\" name=\"nein\" id=\"nein\" value=\"Abbrechen\">";
      echo "<input type=\"submit\" name=\"ja\" id=\"ja\" value=\"Transloaden\"></td></tr>";
      echo "</table>";
      echo "</td></tr></table>";
      echo "</form>";
Warum bei sowas nicht kurz die PHP Tags unterbrechen?
Oder statt zwanzig Echobefehle nur einen machen?
Auch wenn echo relativ schnell ist, hier könntest du eindeutig optimieren.

Dann wäre natürlich die nächste Frage, warum alles in eine Datei?
1600 Zeilen sind:
  • unübersichtlich
  • schwer zu warten
  • unübersichtlich
  • Programmlogik und Anzeigelogik sind vermischt
  • unübersichtlich
Manchmal wiederhole ich mich etwas ;)

Ich müsste für mich ehrlich sagen, dass ich es nie benutzen würde.
Vielleicht holst du dir eine Impressionen von Dennis FTP Klasse?
http://php-classes.sourceforge.net

/edit: Achja, wenn ich das Skript ausführe, kriege ich irgendwo eine Endlosschleife und er überschreitet die max_execution_time.
/edit2: [phpf]error_reporting[/phpf] ganz am Anfang des Skriptes mal auf E_ALL stellen... Dann siehst du noch einige kleine Fehler.
 
Zuletzt bearbeitet:
Oje, das ist ja wirklich ein venichtendes Urteil. Leider kann ich mit error_reporting() nicht arbeiten, da ich PHP nur online zur Verfügung habe. Welche Fehler wären es, die angezeigt werden?

Zum Thema Endlosschleife möchte ich gleich eine Befürchtung äußern: ist es nicht zulässig, eine Funktion sich selbst wieder aufrufen zu lassen? So habe ich z. B. die Baumfunktion aufgebaut, tree_dump() ruft sich immer wieder selbst auf, bis alle Verzeichnisse ausgelesen und -gegeben wurden.
 
Doch, rekursive Funktionen sind auch in PHP erlaubt. Sofern sie irgendwann auch enden und nicht die max_execution_time überschreiten ;)

Also, hier mal ein Auszug, wenn ich debugge:
Debug session started.
Notice: D:\Downloads\weltftp.php line 52 - Undefined index: server
Notice: D:\Downloads\weltftp.php line 53 - Undefined index: benutzer
Notice: D:\Downloads\weltftp.php line 54 - Undefined index: passwort
Notice: D:\Downloads\weltftp.php line 55 - Undefined index: server2
Notice: D:\Downloads\weltftp.php line 56 - Undefined index: benutzer2
Notice: D:\Downloads\weltftp.php line 57 - Undefined index: passwort2
Notice: D:\Downloads\weltftp.php line 58 - Undefined index: verzeichnis
Notice: D:\Downloads\weltftp.php line 59 - Undefined index: ordner
Notice: D:\Downloads\weltftp.php line 60 - Undefined index: datei
Notice: D:\Downloads\weltftp.php line 61 - Undefined index: xordner
Notice: D:\Downloads\weltftp.php line 62 - Undefined index: xdatei
Notice: D:\Downloads\weltftp.php line 63 - Undefined index: xdata
Notice: D:\Downloads\weltftp.php line 64 - Undefined index: xordner2
Notice: D:\Downloads\weltftp.php line 65 - Undefined index: xdatei2
Notice: D:\Downloads\weltftp.php line 66 - Undefined index: xdata2
Notice: D:\Downloads\weltftp.php line 67 - Undefined index: neu
Notice: D:\Downloads\weltftp.php line 68 - Undefined index: chmodnum
Notice: D:\Downloads\weltftp.php line 69 - Undefined index: transziel
Notice: D:\Downloads\weltftp.php line 70 - Undefined index: transfer
Debug Warning: D:\Downloads\weltftp.php line 72 - ftp_login() expects parameter 1 to be resource, boolean given
Debug Warning: D:\Downloads\weltftp.php line 74 - ftp_login() expects parameter 1 to be resource, boolean given
Notice: D:\Downloads\weltftp.php line 78 - Undefined index: REMOTE_ADDR
Notice: D:\Downloads\weltftp.php line 80 - Undefined index: SCRIPT_URI
Debug session ended.

Also entweder fehlt da noch ein Formular was dazu gehört oder es ist einfach fehlerhaft ;)
 
Das sind die Variablen, die durch die Formulareingaben des Nutzers bestimmt werden. Wahrscheinlich laufen die rekursiven Funktionen, die diese Variablen verwenden, deswegen ins Leere.

Übrigens werden in der Benutzeroberfäche die Buttons im Zusammenhang mit cURL-Aktionen zur Vermeidung von fatal error ausgeblendet, wenn kein cURL installiert ist. Ohne cURL kann man allerdings nicht so viel anstellen mit dem Programm.
 
Dann würde ich dir zwei Dinge raten:

Bau dir eine lokale Testumgebung auf, damit du es bequemer hast.

Z. B. mit xAMPP.

Zweitens deklariere die Variablen auch nur wenn das Formular abgeschickt wurde.
Denn sonst wird es auch weiterhin diese Fehlermeldungen geben.
 
Habe wegen register globals off alle betroffenen Variablen nach dem Muster
PHP:
$server = $_POST['server'];
$benutzer = $_POST['benutzer'];
//usw.
deklariert. Ist das der Fehler? Wäre es besser, alle betroffenen Variablen $variable im Code durch $_POST['variable'] zu ersetzen? Leider habe ich zurzeit nur Smartphone, keinen eigenen Computer.
 
Das Problem ist halt, wenn die Variable nicht in $_POST steht, also das Formular nicht abgeschickt wurde, entsteht eine Warning. Eben dass die Variable nicht gesetzt ist.
Umgehen könntest du es so:

PHP:
$server = ( !empty($_POST['server']) ? $_POST['server'] : '' );

Dies ist eine Kurzform, ausgeschrieben würde es so aussehen:

PHP:
if(!empty($_POST['server'])) 
{
  $server = $_POST['server'];
}
else
{
  $server = '';
}

Und dann solltest du, bevor du die Ressource mit ftp_connect aufbaust, prüfen ob $server leer ist.
Falls ja, lohnt es sich ja gar nicht erst ide Verbindung aufzubauen.
 
Hallo,

habe das Script nicht getestet, daher kann ich keine Aussage zu den Warnungen geben.
Bin nur mal über den Code drüber geflogen und muss da Felix in allen Punkt recht geben.

Gerade der Punkt Dokumentation fehlt komplett. Zumal auch deine Variablen gerade bei den Funktionen immer $x, $y heissen.
Hier mal am Beispiel der doch kurzen Funktion win_slashes()

Dein Original
PHP:
function win_slashes($x) {
  global $ds;
  if (strtoupper(substr(PHP_OS, 0, 3)) == "WIN") {
    $y = $ds . $x;
  } else {
    $y = $x;
  }
  return $y;
}
Okay, $x ist der Wert an dem evtl. ein Slash vorgestellt wird. Das kann man noch aus der Länge der Funktion herauslesen.
Aber $ds dachte ich erst wäre DataSource oder sowas in der Richtung. Es ist aber nur eine Variable um die Konstante DIRECTORY_SEPARATOR zu halten (sinn?)
Hier wie ich es vielleicht machen würde
PHP:
/**
 * Prepend a slash to the given path if it's a WIN machine
 *
 * @param string $sPath a file system path
 * @return string with a prepended slash if its a WIN server
 */
function win_slashes($sPath) {
  return DIRECTORY_SEPARATOR == '\\' ? DIRECTORY_SEPARATOR.$sPath : $sPath:
}
Ist jetzt nur ein Beispiel von meiner Seite.
Was ich aber damit zeigen möchte, das vor der Funktion beschrieben steht, was macht die Funktion und über phpDOC Tags die Parameter und der Rückgabewert beschrieben werden.
Zudem habe ich $x gegen das getauscht, was dort bearbeitet wird $sPath. Das "s" deutet an, hier soll in der Variable ein String stehen und "Path" das der Inhalt eine Pfadangabe ist.
Das $ds habe ich komplett rausgenommen und direkt mit der Konstante gearbeitet.

Um das nur nochmal deutlich herauszustellen:
Ich möchte deine Arbeit nicht schlecht reden oder es "vernichtend" beurteilen. Es sind wirklich nur Anregungen, wie man es vielleicht besser machen kann (aus meiner Sicht).
Habe es wir oben gesagt nicht getestet, vielleicht ist da echt eine gute Umsetzung dabei, aber die unübersichtlichkeit...

Um dann mein Posting doch positiv abzuschliessen:
Ein Code, auf dem man auf jeden Fall gut aufbauen kann und Verbesserungen einbringen kann.

Das war es erstmal,
Gruss
 
Zurück