Externe Dateien prüfen

[迪拉斯]

Hey,
einige Sachen, die ich anspreche, sind Mäkelei und haben keine schlimme Wirkung, wenn man sie nicht befolgt. Andere wiederum sind dicke Hunde und können kritisch sein. Zu welcher Kategorie eine gehört, merkst du mit der Erfahrung.

• Codegestaltung ist ziemlich knorrig, lege dir Perl::Tidy zu.
• Du hast vergessen, Taintmode einzuschalten.
• Cargokultprogrammierung mit use. Du musst dir bewusst machen, was use bewirkt, nämlich den Import von Funktionen. Für CGI.pm importierst du :standard, also eine ganze Gruppe von Funktionen, und dann benutzt du sie nicht, dein ganzes HTML ist nämlich manuell zusammengekleistert. Wieso die unnötige Namensraumverschmutzung schlecht ist, bemerkt man dann, wenn man feststellen muss, dass sowohl LWP::Simple als auch CGI.pm eine Funktion namens head exportieren. Geh solchen Kollisionen aus dem Weg, indem du objektorientierte Methodenaufrufe machst, statt Funktionen zu importieren und prozedural aufzurufen. Wenn du importierst, schränke das auf die Funktionen ein, die du wirklich benötigst. Ein Vorteil davon ist, dass im Kopf eines Programms die verwendeten Funktionen quasi durch den Import selber dokumentiert werden, sehr wichtig zur Wartbarkeit.
• Wenn man LWP::UserAgent hat, braucht man LWP::Simple nicht mehr.
• Du hast nicht bedacht, dass in Strings, welche durch " begrenzt werden, Variablen interpoliert werden können. Wenn jemand das Programm übernimmt, den String ändert und ein Sigil $@% einfügt, und versäumt, es zu maskieren, gibt's unerwartete Ergebnisse. Deshalb, defensiv programmieren und standardmäßig ' bzw. q zur Begrenzung verwenden.
• Scope einschränken. Definiere nicht einfach alles zu Anfang, was du benötigst. So ist es schlecht nachvollziehbar, wann welche Variable tatsächlich benutzt wird. Definiere Variablen immer erst kurz bevor du sie brauchst. Hilfsvariablen sollen immer temporär sein und ihre Gültigkeit verlieren, wenn sie ihren Dienst verrichtet haben. Man kann dazu nackte Blöcke {}; einsetzen.
• URIs können Großbuchstaben haben! Hier darf lc nicht eingesetzt werden.
• Ich hab nach 20 Variablendeklarationen aufgehört, zu zählen. Für ein 100-Zeilenprogramm ist das viel zu viel. Die Benennung ist größtenteils undurchsichtig und somit unbrauchbar fürs Verständnis.
• Einige Variablen ändern sich nie. Benutze richtige Konstanten.
• Die Arraydatenstrukturen haben doppelte Zwecke; man kann es deutlich in der Quelle sehen, dass sie für Verschiedenes benötigt werden. Trenne Daten nach Zweck.
• Dein Programm hat keine Kommentare. Du musst aufschreiben, was die vielen Regex und Bedingungen auf anschaulicher Ebene bewirken. Jetzt ist es sehr zeitaufwändig, es nachzuvollziehen.
• s/\/+$//g; Schiefezahnstochersyndrom. Wenn du ein / innerhalb eines Regex benötigst, nimm einfach andere begrenzer! s|/+$||g;
• Du setzt beinahe überall den Modifier g in Regex ein, wo er keinen Sinn ergibt. Bitte nochmal in perlretut nachlesen.
• Du musst noch mal die Doku von LWP aufmerksam lesen. Stell dir vor, der Server liefert eine Fehlermeldung und diese ist für gewöhnlich in HTML; somit ist die URI zwar erreichbar, aber nur, was eine Maschine darunter versteht, nicht ein Mensch. Das kann durch Falscheingabe der URI erfolgt sein oder kann durch ganz andere Umstände außerhalb deines Einflusses. Deine absicht war wohl, zu prüfen, ob der URI auch erfolgreich erreichbar ist, dafür gibt es die Methode is_success.
• Einzelbedingungen schreibt man idiomatischer mit nachfolgendem statement modifier. $ziel = "Ist leer" unless $ziel;
• Lerne, wann man runde Klammern braucht (Bedingungen, Vorranggruppierung, Listenkontext/-erzeugung, Methodenaufrufe) und wann nicht (prozedurale Aufrufe).
• Lerne, wozu Schleifen nützlich sind. Diese verketteten Bedingungen werden sehr schnell sehr unhandlich.
• $ziel und $bild werden unnötigerweise zweimal geholt.
• Mache die Stringverkettung lieber explizit: $ausgabe .= 'angehängtes';
• goto verleitet zu Spagettiprogrammierung. Es gibt viele andere Kontrollstrukturen, die allesamt besser geeignet sind. Siehe perlsyn.
• Wozu die Prüfung auf Endungen? Ressourcen im Web sind keine Dateien, und brauchen keine Endungen zu haben. Die Endungen sind nicht ausschlaggebend für den Typ, also ist ein Test darauf sinnlos. Das ist auch der grund, weshalb die Regex $pb =~ s/(.•)\.//g; fehlerhaft ist. Aus http://www.w3.org/Icons/valid-xhtml10 macht sie org/Icons/valid-xhtml10.
• ($size < $isize) && ($size ne 0)
Da hast du hast negative Zahlen nicht bedacht. $size kann hier zwar nie negativ sein, aber übernehme nicht diese Ausdrucksweise aus Gewohnheit nicht für andere Programme. Der mathematische Ausdruck 0 < $size < $isize heißt in perl5 (0 < $size) && ($size < $isize). (In perl6 ist das bequemer.)
• Wozu eigentlich diese seltsame Trennung von $ziel und $bild? Ressource ist Ressource im Web. Das lässt sich prima generalisieren.
• Ich greife noch mal zusammengestückelte Erzeugung von HTML auf. Da du das Grundprinzip von Vorlagen schon intuitiv erfasst hast, nämlich Daten sammeln und am Ende in einem Rutsch ausgeben, sollte es dir nicht schwerfallen, den gedanklich kleinen Sprung zu machen und auf ein richtiges Templatesystem umzusatteln. Das lohnt sich ab zwei Seiten, und hier ist nur eine, aber ich weigere mich, HTML auf eine niederere Art und Weise zu erzeugen. Ich nehme Text::Template, das ist supereinfach zu lernen und doch so mächtig, dass du für lange Zeit kein anderes brauchst.

So, das war's im Detail. Hier geht ausnahmsweise ein Neuschreiben schneller als schrittweises Refactoring. So würde ich es machen:

  
#!/usr/bin/perl -T  
use v5.8; # für Readonly  
use utf8; # wegen eingebettetem HTML  
use strict;  
use diagnostics;  
  
use CGI qw(); # kein Funktionsimport, nur OOP-Methoden verwendet  
use Image::Size qw(imgsize);  
use LWP::UserAgent qw(); # nur OOP  
use Readonly;  
use Text::Template qw();  
  
# --  
  
my $uri = 'http://www.w3.org/Icons/valid-xhtml10';  
  
Readonly my @ALLOWEDT => qw(text/html text/plain text/xml application/xml application/xhtml);  
Readonly my @ALLOWEDI => qw(image/png image/jpeg image/gif);  
  
Readonly my $SIZEX => 320; # muss exakte größe passen  
Readonly my $SIZEY => 240;  
Readonly my $MAXSIZE => 10241, # muss kleiner als das sein  
  
my %t; # Templatevariablen  
  
# --  
  
my $response = LWP::UserAgent->new->get($uri);  
$t{'code'} = $response->code; # HTTP status code  
$t{'msg'} = $response->message; # HTTP status message  
  
if ($response->is_success) {  
    $t{'ct'} = $response->header('Content-Type');  
    if (scalar grep {$t{'ct'} =~ /^$_/} @ALLOWEDT) {  
        $t{'r'} = 'Der Server lieferte die gewünschte Ressource fehlerfrei. Der Typ ist akzeptiert.';  
    } elsif (scalar grep {$t{'ct'} =~ /^$_/} @ALLOWEDI) {  
        my $img = $response->content;  
        my ($x, $y) = imgsize \$img;  
        $t{'x'} = $x;  
        $t{'y'} = $y;  
        $t{'size'} = length $img;  
        if ($x == $SIZEX and $y == $SIZEY and length $img < $MAXSIZE) {  
            $t{'r'} = 'Der Server lieferte die gewünschte Ressource fehlerfrei. Es handelt sich um ein akzeptiertes Bild.';  
        } else {  
            $t{'r'} = 'Der Server lieferte die gewünschte Ressource fehlerfrei. Es ist zwar ein Bild, aber wird nicht akzeptiert.';  
        };  
    } else {  
        $t{'r'} = 'Der Server lieferte die gewünschte Ressource fehlerfrei, aber sie hat keinen akzeptierten Typ.';  
    };  
} else {  
    $t{'r'} = 'Der Server lieferte eine Fehlermeldung oder ist nicht erreichbar.';  
};  
  
# --  
  
my $q = CGI->new;  
print $q->header('text/html;charset=utf-8');  
my $template = Text::Template->new(TYPE => 'STRING', DELIMITERS => ['<%', '%>'], SOURCE => <<'');  
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">  
<html><head><title>Ergebnis</title></head><body>  
<ul>  
    <li>Status: <% $code %></li>  
    <li>Statusbotschaft: <% $msg %></li>  
    <% $OUT .= "<li>Typ: $ct</li>" if defined $ct %>  
    <%  
        if (defined $size) {  
            $kb = sprintf '%.3f', $size / 1024;  
        $OUT .= qq{  
            <li>Breite: ${x}px</li>  
            <li>Höhe: ${y}px</li>  
            <li>Größe: $size Byte, $kb Kilobyte</li>  
        }}  
    %>  
</ul>  
<p><% $r %></p>  
</body></html>  
  
print $template->fill_in(HASH => \%t);  

Der Programmfluss lässt sich supereinfach lesen, es sind nur Verschachtelungen, keine Sprünge. Schau mal, wie ich die Datenstrukturen kommentiert habe, das macht eine Kommentierung des Algorithmus beinahe überflüssig. Deine Hausaufgabe: Passe die Fehlermeldung so an, dass der Nutzer weiß, aus welchem Grund das Bild nicht akzeptiert ist!