Schrift
[thread]6709[/thread]

Archive::RAR...: treibt mich in den Wahnsinn! (Seite 2)

Leser: 19


<< |< 1 2 3 >| >> 28 Einträge, 3 Seiten
Dubu
 2005-02-23 01:10
#51763 #51763
User since
2003-08-04
2145 Artikel
ModeratorIn + EditorIn

user image
Ich habe mir nicht das gesamte Skript auf Funktionsfaehigkeit angeschaut, aber hier ein paar Bemerkungen. Nimm es einfach mal als Anregungen. ;)

Quote
Code: (dl )
1
2
3
4
5
6
7
8
#Anlegen der Variablen und Arrays
my ($inidaten,$ftphost,$ftpuser,@answer,$antwort,@del_rar,$fd,
$pfad,$Dateipfad,@safe_rar,$zeile,$datei_pfad,@safe_datei,@pd,
$dateigroesse_server,$aenderungsdatum_server,$answer,
$anzahl_server,$anzahl_client,$pfad_dateiname,$ftppasswort,
$socket,$imput,$helper,@pfad,$remotehost,@Serverfiles_loeschen,
$remoteport,$sek,$min,$stu,$tag,$mon,$jahr,$wtag,
$programmpfad);

So viele globale Variable sind eigentlich zu viele.
Da bietet es sich an, einiges in einem einzigen, globalen Hash auszugliedern (oder gleich ein Config-Modul anlegen).

Nachtrag: Offensichtlich brauchst du viele davon sowieso nur lokal, dann deklariere sie doch auch lokal.

Quote
Code: (dl )
1
2
tie my @ini,'Tie::File',$ini; #or die print "InI Datei konnte nicht gefunden werden. Programm wird beendet...;"
foreach my $inidaten(@ini)

Wenn du die Datei sowieso zeilenweise durchgehst, brauchst du eigentlich kein Tie::File. Da kannst du sie gleich zeilenweise einlesen.

Quote
Code: (dl )
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
    {
    if ($inidaten=~/Backuppfad\=/i)
        {
        $helper=index($inidaten,"backuppfad\=")+12;
        $pfad=substr($inidaten,$helper);
           chomp($pfad);
        print "$pfad";                    
        }
    elsif ($inidaten=~/remoteport\=/i)
       {
       $helper=index($inidaten,"remoteport\=")+11;
       $remoteport=substr($inidaten,$helper);
           chomp($remoteport);                      
       }
   elsif ($inidaten=~/remotehost\=/i)
...

Daran sind mehrere Dinge unschoen:
- Du brauchst fast den gleichen Code mehrfach hintereinander.
  Das waere schon mal besser mit einer Subroutine gemacht.
- Das jeweilige Schluesselwort ("backuppfad", "remoteport" etc.)
  muss zweimal auftauchen => anfaellig fuer Tippfehler.
- Die Laenge des Schluesselwortes ist fest kodiert.
- Die Regex ist nicht am Zeilenanfang verankert.

Abgesehen davon, dass es auf CPAN sehr schoene Module zur Verarbeitung von Konfig.dateien gibt, kann man die Auswertung auch "von Hand" schoener machen:
Code: (dl )
1
2
3
4
5
6
7
8
9
my %Config;  # Konfigurationshash
open (INI, $ini) or die "Fehler bei Ini-Datei $ini: $!";
while (<INI>) {
   next if /^#/;   # Kommentarzeilen ueberspringen
   chomp;
   if (/^\s*(\w+)\s*=\s*(.*)/) {
       $Config{$1} = $2;
   }
}

Dazu noch ein paar Kommentare in die Inidatei - fertig ist die Laube!

Quote
Code: (dl )
1
2
open (FEHLER, ">Logdatei.log");
print FEHLER "Programm wurde am "."$datum"." um "."$zeit"." gestartet"."\n"|| die print "$!";

Du hast eine laestige Angewohnheit, die ich anfangs in Perl auch hatte. :) Meine Vermutung ist, dass man sie sich bei der Shellprogrammierung einfaengt: Du setzt Variablen immer in Anfuehrungszeichen, wenn du ihren Wert haben moechtest, in einer Zuweisung oder bei print.
Du schreibst
$var2 = "$var1";
statt
$var2 = $var1;

In dieser Zeile schreibst du die "$!" statt einfach die $!.
Umgekehrt machst du die print-Anweisung komplizierter als noetig.
print FEHLER "Programm wurde am $datum um $zeit gestartet\n" or die print "$!";
ist kuerzer und einfacher.

Quote
Code: (dl )
1
2
3
4
5
#Aufruf der Subroutine.
find(\&wanted,$pfad);
print "test";
#Aufbauen der FTP-Verbindung
my $ftp = Net::FTP->new("$ftphost",

Hier wieder ein Beispiel: Es gibt keinen Grund, um $ftphost Anfuehrungszeichen zu setzen.

Quote
Code: (dl )
if ($answer =~ "keine Zugriffsberechtigung")

Wo etwas als regulaerer Ausdrueck interpretiert wird, sollte man das auch durch die Schreibweise klar machen. Anfuehrungszeichen sind hier also schlecht:
Code: (dl )
if ($answer =~ /keine Zugriffsberechtigung/)


Quote
Code: (dl )
1
2
#Sendet den Pfad an den Server;
print $socket "$pfad"."\n";

Das kann einfach
print $socket "$pfad\n";
heissen.

Quote
Code: (dl )
1
2
$anzahl_client = 0;
todo:

Die Sprungmarke brauchst du offensichtlich nicht.

Quote
Code: (dl )
while ($$anzahl_client eq $anzahl_server)

Was soll $$anzahl_client sein? $anzahl_client ist doch eine Zahl, keine Referenz. Das muesste also eine Warnung geben.

Abgesehen davon kommt mir die Schleifenbedingung sowieso etwas merkwuerdig vor. Meintest du vielleicht
while ($anzahl_client < $anzahl_server)
? Mit "eq" vergleicht man auch keine Zahlen, sondern Strings. Bei Zahlen wuerde man mit "==" auf Gleichheit testen.

Quote
Code: (dl )
   $anzahl_client = $anzahl_client + 1;

Kuerzer:
++$anzahl_client;

Quote
Code: (dl )
    $daten{dateien}{"$pfad_dateiname"}{'aenderungsdatum_server'}="$aenderungsdatum_server";

$daten{dateien}{$pfad_dateiname}{aenderungsdatum_server} = $aenderungsdatum_server;
etc.

Quote
Code: (dl )
1
2
3
4
foreach (sort keys %{$daten{dateien}})
    {
    push (@pd,$_);
    }

Statt der Schleife:
Code: (dl )
@pd = sort keys %{$daten{dateien}};


Quote
Code: (dl )
foreach my $zeile(@pd)

Nun gut, du brauchst @pd eigentlich gar nicht, sondern kannst gleich hier sort keys ... einsetzen.

Quote
Code: (dl )
1
2
3
4
    unless ($daten{dateien}{"$zeile"}{'aenderungsdatum_client'})
       {
       push (@Serverfiles_loeschen, "$zeile");
       }

Auch hier sind wieder viele ueberfluessige Anfuehrungszeichen. Andererseits die Frage: Willst du wirklich testen, ob das Aenderungsdatum ungleich Null ist, oder ob es im Hash existiert? Fuer letzteres gibt es exists().
Code: (dl )
1
2
3
unless (exists $daten{dateien}{$zeile}{aenderungsdatum_client}) {
   push @Serverfiles_loeschen, $zeile;
}


Quote
Code: (dl )
        elsif ($daten{dateien}{"$zeile"}{'dateigroesse_client'} ne $daten{dateien}{"$zeile"}{'dateigroesse_server'})

Hier meinst du bestimmt auch "!=" statt "ne" (auch wenn es meist keinen Unterschied macht).

Quote
Code: (dl )
1
2
3
4
5
6
7
 $datei_pfad=$pfad.$zeile;
               
 $datei_pfad=~ tr/\//\\/;  
               
 push (@safe_datei,"\"$datei_pfad\"")
               
 }

Auch hier sind wieder viele Wiederholungen.
Ich haette die ganze Schleife wahrscheinlich so aehnlich geschrieben:
Code: (dl )
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
@safe_datei = ();
for my $zeile (sort keys %{$daten{dateien}})
{
   my $zeilendaten = $daten{dateien}{$zeile};
   push @Serverfiles_loeschen, $zeile
       unless exists $zeilendaten->{aenderungsdatum_client};
   if ($zeilendaten->{dateigroesse_client} !=
       $zeilendaten->{dateigroesse_server} or
       $zeilendaten->{aenderungsdatum_client} ne
       $zeilendaten->{aenderungsdatum_server})
   {
       my $datei_pfad = $pfad . $zeile;
       $datei_pfad =~ tr~/~\\~;
       push @safe_datei, qq{"$datei_pfad"};
   }
}

Dabei habe ich soweit wie moeglich Variablen lokal beschraenkt ($zeile, $datei_pfad), die somit nicht mehr global deklariert werden muessen. Ausserdem habe ich die ganzen if-Anweisungen in einer - soweit ich sehe gleichwertigen - zusammengefasst und damit keine Codewiederholungen mehr.

Quote
Code: (dl )
    print FEHLER "Update ist nicht erfordelerlich. Programm wurde beendet."."\n";

"erforderlich"

Quote
Code: (dl )
1
2
my $dateienfertig = @safe_datei;
while (my $dateiengesamt ne my $dateienfertig)

Du deklarierst unabsichtlich $dateienfertig in der Schleife neu und weist vorher an $dateienfertig statt $dateiengesamt zu.
Du meinst wahrscheinlich:
Code: (dl )
1
2
3
my $dateiengesamt = @safe_datei;
my $dateienfertig = 0;
while ($dateienfertig < $dateiengesamt)


Quote
Code: (dl )
1
2
        {
        foreach (@safe_datei)

Damit kannst du dir die aeussere while-Schleife wiederum sparen, oder moechtest du mehrfach durch @safe_datei gehen?

Quote
Code: (dl )
            if ($counterdatei ne 900)

Auch hier:
if ($counterdatei < 900) ...

Quote
Code: (dl )
 my $i++;

Hm. Das heisst soviel wie my $i = 1.
Falls du $i hochzaehlen moechtest, musst du es ausserhalb der Schleife deklarieren.
               
Quote
Code: (dl )
     -archive => "pre_rar"."$i",

Code: (dl )
-archive => "pre_rar$i",


Quote
Code: (dl )
1
2
3
4
foreach $zeile (@del_rar)
{
unlink($zeile);
}

Code: (dl )
unlink @del_rar;


Quote
Code: (dl )
1
2
3
4
#subroutine zur suche der erforderlichen rar_dateien.
sub wanted_rar
    {
    if (($_ =~ ".rar")&&($_ =~ "pre_rar\."))

Das macht evtl. nicht das, was du meinst. Die erste Regex findet jede Datei, die irgendwo ein beliebiges Zeichen gefolgt von "rar" im Namen hat, also auch "ferrari.jpg" oder "Mehrarbeit.zip"; die zweite findet alle Dateien mit "pre_rar." irgendwo. Das sind zumindest nicht die Dateien, die du angelegt hast, denn die heissen doch eher "pre_rar<nummer>.rar", oder? Auch sollte man - wie oben schon gesagt - Regexen immer als solche schreiben, damit man weiss, dass man auf Sonderzeichen achten muss.
Code: (dl )
if (/^pre_rar\d+\.rar$/) ... # Vorne pre_rar, gefolgt von einer Zahl, gefolgt von .rar am Ende


Quote
Code: (dl )
1
2
3
4
5
6
7
8
#subroutine zur suche der erforderlichen Dateien
sub wanted
    {
    if( -f $File::Find::name)
        {    
        $helper=length($pfad);
        $Dateipfad=substr($File::Find::name,$helper);
        chomp ($Dateipfad);

Ueberflussig, es gibt $File::Find::dir.

Quote
Code: (dl )
1
2
3
4
5
6
        $ctime_cl=scalar localtime ((stat $File::Find::name)[9]);
        my @split_cttime_cl = split (/ /,$ctime_cl);
        my @split_time = split (/:/, $split_cttime_cl[3]);
        $ctime_cl = "$split_cttime_cl[0]"." "."$split_cttime_cl[1]"."
"."$split_cttime_cl[2]"." "."$split_time[0]".":"."$split_time[0]"."
"."$split_cttime_cl[4]";

Das sieht grauslich aus, vor allem wenn man sieht, wie schoen du das in den Routinen time() und date() geloest hast. Warum nimmst du hier "scalar localtime (...)", statt gleich mit dem Array zu arbeiten und mit sprintf(), wie unten?

Ausserdem benutzt du zweimal $split_time[0] beim Zusammensetzen.

Quote
Code: (dl )
        $daten{dateien}{"$Dateipfad"}{'aenderungsdatum_client'}="$ctime_cl";

Nochmal zur Erinnerung:
Code: (dl )
$daten{dateien}{$File::Find::dir}{aenderungsdatum_client} = $ctime_cl;


Ich hoffe, ich konnte dir mit den Kommentaren etwas weiterhelfen.
zipster
 2005-02-23 09:04
#51764 #51764
User since
2004-09-06
458 Artikel
BenutzerIn
[default_avatar]
@Dubu
Erstmal vielen Dank...

WoW was du dir für ne Mühe gegeben hast. ;)

Man merkt wohl ganz stark an dem Code das ich eigentlich noch ein blutiger Anfänger bin ;)

Ich werde mal deine Tips umsetzten...

Vielen Dank nochmal

/EDIT

Ich hab noch einige Fragen zu deinen Verbesserungvorschlägen

Ich hoffe du kannst mir nochmal ein wenig weiterhelfen

Code: (dl )
1
2
3
4
5
...
if (/^\s*(\w+)\s*=\s*(.*)/) {
$Config{$1} = $2;
}
...


Was passiert da genau? Ich verstehs noch nicht so ganz.

Wie müßte da meine InI Datei aussehen?
So ungefähr?
Code: (dl )
1
2
3
4
5
#Programmpfad
C:\tools\client
#Backuppfad
Z:\Installs
...


Ist das
Code: (dl )
1
2
3
...
$datei_pfad =~ tr~/~\\~;
...

das selbe wie das?
Code: (dl )
1
2
3
...
$datei_pfad=~ tr/\//\\/;
...


Wenn ja warum machst du da "~" hin? Was haben die da für ne Wirkung?


Code: (dl )
push @safe_datei, qq{"$datei_pfad"};


qq?
Bedeutet das das er alles was in der {} steht genau so in das array schiebt?

Code: (dl )
if (/^pre_rar\d+\.rar$/) ... # Vorne pre_rar, gefolgt von einer Zahl, gefolgt von .rar am Ende


Wie müßte das ausehen wenn die Datei "pre_rar1.part001.rar" heißt? Wobei ich nicht weiß wieviel Parts es gibt.
Wäre das richtig?
Code: (dl )
if (/^pre_rar\d+\.part\d+\.rar$/)



Quote
Das sieht grauslich aus, vor allem wenn man sieht, wie schoen du das in den Routinen time() und date() geloest hast. Warum nimmst du hier "scalar localtime (...)", statt gleich mit dem Array zu arbeiten und mit sprintf(), wie unten?


Weil ich mir diese 2 Routinen aus einem anderen Code geliehen habe. Ehrlich gesagt habe ich aber keine Ahung wie genau das funktioniert... :)\n\n

<!--EDIT|zipster|1109145604-->
Dubu
 2005-02-23 10:41
#51765 #51765
User since
2003-08-04
2145 Artikel
ModeratorIn + EditorIn

user image
[quote=zipster,23.02.2005, 08:04]
Man merkt wohl ganz stark an dem Code das ich eigentlich noch ein blutiger Anfänger bin ;)
[/quote]
Na, dafuer ist es schon mal nicht schlecht. :)

Quote
Code: (dl )
1
2
3
4
5
...
if (/^\s*(\w+)\s*=\s*(.*)/) {
      $Config{$1} = $2;
  }
...


Was passiert da genau? Ich verstehs noch nicht so ganz.

Die Regexp bedeutet folgendes:
Anfang der Zeile, gefolgt von beliebig vielen (inkl. gar keinen) Leerzeichen oder Tabs, zusammen kurz Whitespace, gefolgt von einer Folge von Wortzeichen (A-Z, a-z, 0-9 oder _), die ich mir mit den runden Klammern drumherum merke. Dies wiederum gefolgt von beliebig Whitespace, dann einem Gleichheitszeichen, dann wieder beliebig Whitespace, dann eine beliebige Zeichenfolge, die auch leer sein kann.

Quote
Wie müßte da meine InI Datei aussehen?
So ungefähr?
Code: (dl )
1
2
3
4
5
#Programmpfad
C:\tools\client
#Backuppfad
Z:\Installs
...

Nein, sie muesste eigentlich wie vorher aussehen, nur
- kann sie Kommentare enthalten; die muessen nur am
  Zeilenanfang mit '#' beginnen
- koennen am Anfang der Zeile und um das Gleichheitszeichen
  herum Leerzeichen oder Tabs stehen.

Deine Ini-Datei koennte also so aussehen:
Code: (dl )
1
2
3
4
5
# Hier kommt der Pfad zu den Programmen:
programmpfad = C:\tools\client
# Dies ist der Pfad zu den Backups
backuppfad = Z:\Installs
# etc. ...


Quote
Ist das
Code: (dl )
1
2
3
...
$datei_pfad =~ tr~/~\\~;
...

das selbe wie das?
Code: (dl )
1
2
3
...
$datei_pfad=~ tr/\//\\/;
...


Wenn ja warum machst du da "~" hin? Was haben die da für ne Wirkung?

Die Such- und Ersetzungoperatoren m//, s/// und tr/// koennen fast beliebige andere Begrenzer haben als den Schraegstrich (/). In dem Fall oben finde ich es besser, keinen Schraegstrich als Begrenzer zu benutzen, weil er schon im Suchteil vorkommt und dort dann mit \ escaped werden muss. Mit der Tilde als Begrenzer sieht es IMHO etwas uebersichtlicher aus, das ist alles.

Man kann auch Klammern als Begrenzer nehmen, das ist manchmal auch besser verstaendlich.

Folgende Konstruktionen sind von der Funktion her alle gleich:
Code: (dl )
1
2
3
4
5
6
tr/\//\\/
tr~/~\\~
tr!/!\\!
tr./.\\.
tr{/}{\\}
tr(/)(\\)

Siehe dazu perlop, Abschnitt "Quote and Quote-like Operators".

Quote
Code: (dl )
push @safe_datei, qq{"$datei_pfad"};


qq?
Bedeutet das das er alles was in der {} steht genau so in das array schiebt?

In Perl kann man zum Quoten von Text nicht nur '' und "" benutzen, sondern auch die Operatoren q{} und qq{}, wobei der erste wie einfache Anfuehrungszeichen wirkt und der zweite wie doppelte. Fuer die Begrenzer (hier die geschwungenen Klammern) gilt das Gleiche wie bei tr/// oben: Man kann fast alles dafuer einsetzen.

Folgende Ausdruecke sind also alle gleich:
Code: (dl )
1
2
3
"Hallo, \"Welt\"!"
qq{Hallo, "Welt"!}
qq~Hallo, "Welt"~

Je nachdem, welches Zeichen garantiert nicht im Text vorkommt, kann man einen anderen Begrenzer waehlen.

Quote
Code: (dl )
if (/^pre_rar\d+\.rar$/) ... # Vorne pre_rar, gefolgt von einer Zahl, gefolgt von .rar am Ende


Wie müßte das ausehen wenn die Datei "pre_rar1.part001.rar" heißt? Wobei ich nicht weiß wieviel Parts es gibt.
Wäre das richtig?
Code: (dl )
if (/^pre_rar\d+\.part\d+\.rar$/)

Ja, gut!

Quote
Quote
Das sieht grauslich aus, vor allem wenn man sieht, wie schoen du das in den Routinen time() und date() geloest hast. Warum nimmst du hier "scalar localtime (...)", statt gleich mit dem Array zu arbeiten und mit sprintf(), wie unten?


Weil ich mir diese 2 Routinen aus einem anderen Code geliehen habe. Ehrlich gesagt habe ich aber keine Ahung wie genau das funktioniert... :)

[/quote]
Schau dir mal perldoc -f localtime an. Die Funktion localtime() liefert naemlich im Arraykontext eine Liste von einzelnen Werten fuer Datum und Uhrzeit. Im skalaren Kontext (den du oben mit "scalar" erzwungen hast) bekommt man dagegen einen String mit formatiertem Datum und Uhrzeit.

So, ich muss jetzt leider weg.  Hoffe geholfen zu haben. :)
zipster
 2005-02-23 12:18
#51766 #51766
User since
2004-09-06
458 Artikel
BenutzerIn
[default_avatar]
Quote
So, ich muss jetzt leider weg. Hoffe geholfen zu haben. :)


Auf jeden Fall!!! :D

Sowas könnte ich öfter gerauchen, heute habe ich nämlich mal ne Menge dazu gelernt.

Ich denke zwar nicht das der Fehler dadurch behoben ist, aber ich hab wenigstens im Programmieren dazu gelernt :laugh: :laugh: :laugh:
zipster
 2005-02-23 13:57
#51767 #51767
User since
2004-09-06
458 Artikel
BenutzerIn
[default_avatar]
Gibt es eine Möglichkeit eine Datei die man in Perl erstellt (open (FILE, >test.txt) ) als UNICODE abzuspeichern?
renee
 2005-02-23 14:17
#51768 #51768
User since
2003-08-04
14371 Artikel
ModeratorIn
[Homepage] [default_avatar]
Schau Dir mal CPAN:PerlIO::encoding oder CPAN:PerlIO::via::Unidecode an...
OTRS-Erweiterungen (http://feature-addons.de/)
Frankfurt Perlmongers (http://frankfurt.pm/)
--

Unterlagen OTRS-Workshop 2012: http://otrs.perl-services.de/workshop.html
Perl-Entwicklung: http://perl-services.de/
zipster
 2005-02-23 14:40
#51769 #51769
User since
2004-09-06
458 Artikel
BenutzerIn
[default_avatar]
Gibts nicht ne möglichkeit ohne externes Modul?
ptk
 2005-02-23 15:07
#51770 #51770
User since
2003-11-28
3645 Artikel
ModeratorIn
[default_avatar]
Seit perl5.8.0 ist alles dabei, was man fuer utf-8 braucht... (Stichwoerter: Encode, encoding)
zipster
 2005-02-23 15:09
#51771 #51771
User since
2004-09-06
458 Artikel
BenutzerIn
[default_avatar]
ptk
 2005-02-23 15:22
#51772 #51772
User since
2003-11-28
3645 Artikel
ModeratorIn
[default_avatar]
Code: (dl )
1
2
open my $fh, "> DATEI" or die $!;
binmode($fh, ":encoding(utf-8)");

Statt utf-8 kann man auch andere Encodings verwenden, z.B. braucht die Windows-Konsole (nur auf aelteren Windows-Systemen?) statt iso-8859-1 etwas wie cp437 oder cp850.
<< |< 1 2 3 >| >> 28 Einträge, 3 Seiten



View all threads created 2005-02-15 18:11.