Johann Snyman wrote:
> Hello there,
>
> I am a newby trying to learn pearl. I wrote my first little script,
> and I would love to se where I could have done things better (in a
> more proffesional way).
>
> Feedback would be appreciated!
Well you are off to a good start with lines two and three ;)
> #!/usr/bin/perl
> use strict;
> use warnings;
>
> our @scores = `cat /home/johann/smail/Spam/* | grep ^Subject:` ;
> # string looks like this: Subject: 10.20 Re: Hallo
Good, commenting your code is a good thing :)
However backticks aren't the only way or always the best way ... In this
case it's not a bad usage, but always be aware of when it COULD be. Reading
perldoc perlsec is a good place to observe some security issues. (Running
this under -T isn't a bad idea either.)
observe:
my @scores;
my @files = glob "/home/johann/smail/Spam/*";
foreach my $file (@files) {
open(IN, "<", $file) or die "Cannot open file $file: $!";
while (<IN>) {
next unless /^Subject:/;
push @scores, $_;
last; # found what we need, no need to go further with this file
}
close IN or die "Cannot close filehandle for $file: $!";
}
> our @vals;
> our $counter = 0;
no need to make these globals.. this will do:
my($counter, @vals);
> for (@scores) {
> $_ =~ /([0-9][0-9]\.[0-9][0-9])/;
> $vals[$counter] = $1;
> $counter++;
> }
# and in fact, you don't need $counter at all...
foreach my $score (@scores) {
# this avoids problems in the case of the regex NOT matching
unless ($score =~ /\s([\d]+\.[\d]+)\s/) {# assumes score is space-delimited
warn "$score did not match!";
next;
}
push @vals, $1;
}
> my $saved = `cat /home/johann/.spamscore` || die $!;
> chomp($saved);
> push @vals, $saved;
open IN, "<", "/home/johann/.spamscore"
or die "Cannot open .spamscore file: $!";
while (<IN>) {
chomp;
push @vals, $_ if $_;
}
close IN or die "Could not close .spamscore file: $!";
> @scores = sort(@vals);
> #for (@scores) {
> # print $_,"\n";
> # }
re-using the same array to describe something different isn't always a good
idea. rather,
my @newscores = sort @vals;
#this is also an easier way of writing the above debugging check ;)
#print "$_\n" foreach @newscores;
> my $highest = @scores;
> $highest -= 1;
why not just do:
#my $highest = $#newscores;
#although you don't need to... see below :)
> open (FH, ">/home/johann/.spamscore") || die $!;
>
> print FH $scores[$highest];
>
> close(FH);
this is ok, but again,
open FH, ">", "/home/johann/.spamscore"
or die "Cannot open .spamscore for writing!: $!";
print FH $newscores[$#newscores];
close FH or die "Cannot close output file .spamscore: $!";
> __END__
>
>
> Thanks!
>
useful [descriptive] die error messages can help debug things much more
easily.
This help any?
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
