Das Skript zum Montag - Blocklist.de befragen

[1unitedpower]

Ich kenne den Dienst nicht, aber es gibt scheinbar auch eine bereits eine HTTP-API dafür. Mir ist nicht so recht klar, welchen Vorteil dein Skript demgegenüber bietet.

Dafür wollen die einen Key (Dann kann man aber auch Angreifer dort einwerfen). Für die DNS-Abfrage nicht ...

Für das Abfragen von IP-Adressen braucht man keinen API-Key. Steht dort direkt unter der ersten Tabelle.

Ein paar stilistische Anmerkungen zu deinem Skript hätte ich auch noch:

ini_set('display_errors', 0);

Wieso überschreibst du diese Server-Einstellung? Auf Produktiv-Servern kann man damit rechnen, dass diese Eigenschaft bereits deaktivert ist. Auf Entwicklungs-Servern möchte man Fehler bewusst anzeigen lassen, da erschwert diese Überschreibung nur die Fehlersuche.

Volle Absicht. Macht man das nicht werden die Notizen in der Ausgabe ausgeworfen und der HTTP-Status auf 200 gesetzt. Das genau will ich hier nicht.

Die Frage war, wieso du das in PHP machst und nicht in der PHP-Konfiguration? Das würde dir erlauben, das Skript mit und ohne display_errors auszuführen ohne das Skript selber berühren zu müssen. Auf Entwicklungsservern will man ja gerade Fehler sehen, da bildet dein Skrtip auch keine Ausnahme.

if ( empty($_REQUEST['method'])) {
    $_REQUEST['method'] = 'HOST';
}

$_REQUEST ist nicht der Ort um benutzerdefinierte Werte darin zu speichern.

Du hast nicht erkannt, dass das eigentlich zum Test gehört.

Zum einen würde ich hier keine Ausnahme für Tests machen, zum anderen gehört der Code bereits nicht mehr zum Test, wie der Kommentar "Ende Test" der Zeile unmittelbar davor es belegt.

Wer das produktiv nutzt sollte natürlich den Testbereich rauswerfen und wird sich wohl je nach Nutzung auf eine andere Methode der Übergabe bzw. Fixierung festlegen.

Das ist Benutzerunfreundlich und wird so nur selten geschehen. Dafür müsste ein Anwender ja erst das ganze Skript lesen und verstehen und darüber hinaus bereit sein es zu manipulieren, wohlwissend, dass eventuelle Updates des Original-Autors danach nicht mehr einfach eingespielt werden können. Da lehnen erfahrene Entwickler schos aus Scheu vor dem Wartungsaufwand ab.

if (
    $_REQUEST['method'] != 'PHP'
    && $_REQUEST['method'] != 'NSLOOKUP'
    && $_REQUEST['method'] != 'HOST'
    ) {
    $_REQUEST['method'] = 'HOST';
}

Die Stelle musste ich 3 Mal lesen, um die Klammernpaare einander zuzuordnen.

Hätte nicht gedacht, dass das schwer zu lesen ist. Ich habe die 3 Bedingungen auf 3 Zeilen verteilt, damit der Umbruch nicht durch die Darstellung hier im Forum an willkürlicher Stelle erfolgt - was den Code noch viel schlechter lesbar macht.

Der Umbruch innerhalb der Bedingung trägt weniger zur Verwirrung bei als die Positionierung der Klammern. Alternativ hättest du die Bedinung aber auch so kürzen können, dass ein Umbruch gänzlich überflüssig wird.

if (!in_array($_REQUEST['method'],['PHP','NSLOOKUP','HOST']))

In meinem Styleguide steht: "Brich die Bedingungen um und rücke diese bei Klammerung genau sei ein wie Du es bei Codeblöcken tun würdest." Ist prima lesbar!

Das tust du aber nicht, wieso ist die schließende Klammer der if-Bedingung eingerückt? Bei Code-Blöcken ist die schließende Klammer auf selber Höhe wie der Beginn der öffnenden Zeile. Als ich von Styleguides sprach, dachte ich übrigens an einen etablierten Stylguide. Die haben zum Vorteil, dass man den eigenen PHP-Code automatisiert dagegen validieren kann.

exit; macht es ebenfalls schwer die Struktur von Quelltexten zu erfassen.

Ich komme damit gut klar. trigger_error ("Foo, blah, blubb", USER_ERROR) bricht auch ab und ich muss es bis nach ganz rechts lesen um das zu wissen. Mir signalisiert das exit deutlich genug, was da los ist und ich empfinde es eher als Zumutung, wenn ich womöglich erst 3000 Zeilen und 24 Dutzend elseif weiter unten erst sehe: da kommt nichts mehr.

Bei gut strukturietem Quelltext musst du gar nicht danach fragen, ob noch etwas kommt. Das Programm sollte so strukturiert sein, dass es dir völlig egal sein kann, ob nach einer geschlossenen Sinneinheit noch eine weitere geschlossene Einheit ausgeführt wird. Das ist das Lokalitätsprinzip. exit; ist das Gegenteil davon, nachfolgender Code wird einfach radikal abgeschnitten, "nach mir die Sintflut". Implikationen davon sind, dass Unit-Testing und Post-Processing nicht mehr möglich sind.

Ich schätze, der Interpreter sieht das auch so...

Der Interpreter muss dein Programm nicht sinnerfassend lesen und verstehen. Er muss es nur stumpf ausführen, das ermächtigt ihn dazu selbst Programmiersprachen mit Leichtigkeit auszuführen, die für den Menschen völlig unverständlich sind, z.B. Brainfuck. Als Maßstab für deinen Programm-Entwurf sollten kognitive Fähigkeiten des Menschen dienen.

    $return['Hint']             = 'Dies ist ein Test! Um eine stabile API zu erhalten fragen Sie beim Autor nach (www.fastix.org)';

Diser Kommentar verunsichert nur. Entweder würde ich hier direkt dokumentieren ...

Was bedeutet wohl, dass das Skript im Ordner /test/ ist? Könnte beides zusammen die sehr deutliche Nachricht beinhalten, dass ich so klar wie möglich machen will, dass niemand darauf vertrauen soll, dass das Skript auch übermorgen noch dort ist und so wie bisher funktioniert?

Ah, dies ist also ein Hinweis an die Nutzer deiner Demo. Ich hatte ihn erst als einen Hinweis an den Anwendungsentwickler wahrgenommen.

Im übrigen unterscheiden sich diese Codeblöcke doch etwas zu sehr als dass ich hier von einer "wiederverwendbaren eigenen Einheit" ausgehen würde.

Kennst du das DRY-Prinzip? Defakto kannst du jedes Programm so refaktorisieren, dass sich keine zwei Zeilen mehr gleichen müssen. Das führt zu erstaunlich eleganten Programmentwürfen. Voraussetzung ist natürlich, dass man die richtigen Abstraktionen wählt. Du musst das Prinzip ja nicht gleich bis ins Extrem betreiben, aber eine Annäherung daran würde deinen Programmen sicher gut tun. Aktuell machst du von Abstraktion und Wiederverwendung überhaupt keinen Gebrauch und stehst somit beim gegenüberliegendem Extrem.

Die anderen Fehlermeldungen beziehen sich auf die Ausführungsumgebung: Existiert dieses Programm?

Ist hier auch drin.

Das war die Kritik. Denn "hier" gehören sie nicht hin.

Du hast dir sicher etwas dabei gedacht, diese und andere Variablen groß zu schreiben.

Der ursprüngliche Plan sah Konstanten vor. Davon bin ich abgerückt, weil PATTEN_SONSTWAS irgendwie zu allgemein ist. Wem das nicht passt, der kann mit "Suchen und Ersetzen" daraus machen was in seinem Styleguide steht.

Wem das nicht passt, der wird nicht in die Pflege deines Skriptes einsteigen, der wird einfach davon Abstand nehmen. Das kann ja auch nicht in deinem Sinn sein.