On Mon, 3 Mar 2003, Scott R. Godin wrote:

>
> 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: $!";
> }

I noticed that you've said
"open () or die()"
and
"close() or die()"

If open fails, the program will kill itself, so the close function
will never be called.
Therefore there is no need to say "close() or die()"

You've done it thrice in this email, so I'm presuming
that it's a habitual thing.

Putting it won't hurt, but it's not useful either.

Just thought I should point it out :)

George P.


>
> > 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]
>
>


-- 
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to