Opened by jan999 at 2009-04-02 07:58
User since
2003-08-04
2145 Artikel
ModeratorIn + EditorIn
2003-08-04
2145 Artikel
ModeratorIn + EditorIn
Zum einen möchte ich die Anmerkungen von "wer" unterstützen; das Einlesen ist wahrscheinlich die Fehlerquelle, das Parsen von XML sollte man anders machen.
Zum anderen ist der Code auch an anderen Stellen ziemlich grauslich. Offensichtlich werden globale Variablen zur Übergabe von Werten zwischen subs genutzt ($line, $isBz2), was sehr fehlerträchtig ist.
Dann scheint der Autor den Match-Operator nicht zu kennen, sondern nutzt stattdessen grep(), was dafür eigentlich nicht gedacht ist:
Dabei ist im ersten Fall auch noch die Regexp falshc. Sie passt auf jeden Dateinamen, der irgendwo hinter einem beliebigen Zeichen die Zeichenkette "bz2" enthält, also auch "abz2weg.html" o.ä.
Besser formuliert wäre es so:
Man kann die obere Zeile, wenn man will, auch noch durch eine etwas perligere Version ersetzen. Und im unteren Fall ist bei großen Datenmengen ein index() bestimmt schneller als eine hier überflüssige Regexp:
Das nur als Anmerkung, funktionieren wird es (in den meisten Fällen) auch so. Vielleicht sollte ich mich mal mit dem Autor in Verbindung setzen.
Zum anderen ist der Code auch an anderen Stellen ziemlich grauslich. Offensichtlich werden globale Variablen zur Übergabe von Werten zwischen subs genutzt ($line, $isBz2), was sehr fehlerträchtig ist.
Dann scheint der Autor den Match-Operator nicht zu kennen, sondern nutzt stattdessen grep(), was dafür eigentlich nicht gedacht ist:
Code (perl): (dl
)
1 2 3
if (grep /.bz2/, $fileName) { $isBz2 = 1 ; } else { $isBz2 = 0 ; } ... while ( ! (grep /node/, $line) ) {
Dabei ist im ersten Fall auch noch die Regexp falshc. Sie passt auf jeden Dateinamen, der irgendwo hinter einem beliebigen Zeichen die Zeichenkette "bz2" enthält, also auch "abz2weg.html" o.ä.
Besser formuliert wäre es so:
Code (perl): (dl
)
1 2 3
if ($fileName =~ /\.bz2$/) { $isBz2 = 1 ; } else { $isBz2 = 0 ; } ... while ($line =~ /node/) { ...
Man kann die obere Zeile, wenn man will, auch noch durch eine etwas perligere Version ersetzen. Und im unteren Fall ist bei großen Datenmengen ein index() bestimmt schneller als eine hier überflüssige Regexp:
Code (perl): (dl
)
1 2 3
$isBz2 = ($fileName =~ /\.bz2$/) ? 1 : 0 ; ... while (index($line, 'node') >= 0) { ...
Das nur als Anmerkung, funktionieren wird es (in den meisten Fällen) auch so. Vielleicht sollte ich mich mal mit dem Autor in Verbindung setzen.