DISCLAIMER: These are just my opinions, and are hardly facts (and I figure
many will disagree with me).
I don't see anything that could be considered the "wrong" way, but here are
a few things you could have done...
# your code
for (@scores) {
$_ =~ /([0-9][0-9]\.[0-9][0-9])/;
$vals[$counter] = $1;
$counter++;
}
# alternate
for (@scores) {
/(\d\d\.\d\d)/; # "$_ =~" is implied, use \d for digit
$vals[$counter++] = $1; # two statements in one
}
# your code
our @scores = `cat /home/johann/smail/Spam/* | grep ^Subject:` ;
# alternate - doesn't depend on cat/grep, works on any OS
our @scores = ();
for (</home/johann/smail/Spam/*>) {
open IN, $_ or next; # skips if file can't be opened
push @scores, grep {/^Subject/} (<IN>);
close IN;
}
# alternate - more elaborate, handles both of above
our @vals = ();
for (</home/johann/smail/Spam/*>) {
open IN, $_ or next; # skips if file can't be opened
push @vals, map {/(\d\d\.\d\d)/} grep {/^Subject/} (<IN>);
close IN;
}
This last one *should* (i.e. untested) read in from the file, grep lines
starting with "Subject", match "\d\d\.\d\d", and add the matched result to
@vals. This removes your counter, removes @scores, and will hopefully be
faster (not sure). It should also work on any OS that it is run on.
Then you have this...
# your code
my $highest = @scores;
$highest -= 1;
# alternate
my $highest = $#scores; # returns last element index
# more of your code
open (FH, ">/home/johann/.spamscore") || die $!;
print FH $scores[$highest];
close(FH);
# alternate - replaces both parts, no variable $highest
open (FH, ">/home/johann/.spamscore") || die $!;
print FH $scores[-1]; # "-1" is a shortcut for 1st from the end
close(FH);
# alternate - same but includes the sort as well (uses @vals directly)
open (FH, ">/home/johann/.spamscore") || die $!;
print FH ((sort @vals)[-1]);
close(FH);
# the whole thing (untested!)...
#!/usr/bin/perl
use strict;
use warnings;
our @vals = (); # decimal values
# open matching files, extract values from subject
for (</home/johann/smail/Spam/*>) {
open IN, $_ or next; # skips if file can't be opened
push @vals, map {/(\d\d\.\d\d)/} grep {/^Subject/} (<IN>);
close IN;
}
# save highest value to disk
open (FH, ">/home/johann/.spamscore") || die $!;
print FH ((sort @vals)[-1]);
close(FH);
Again, there is no real right/wrong way, and it depends on what you are
trying to accomplish. If your goal is readability, then you may wish to
avoid the map/grep I used. If your goal is speed you might (and I am no
expert on this) find that using map/grep is faster. Less variables is
usually better for both readability and performance, so don't create vars
you don't really need (like $highest). Comments are also a good idea, it
will help if you need to modify the code later on.
Rob
-----Original Message-----
From: Johann Snyman [mailto:[EMAIL PROTECTED]
Sent: Wednesday, February 26, 2003 12:42 PM
To: [EMAIL PROTECTED]
Subject: Newby first little script. (Lots of errors and wrong ways of
doing things)
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!
#!/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
our @vals;
our $counter = 0;
for (@scores) {
$_ =~ /([0-9][0-9]\.[0-9][0-9])/;
$vals[$counter] = $1;
$counter++;
}
my $saved = `cat /home/johann/.spamscore` || die $!;
chomp($saved);
push @vals, $saved;
@scores = sort(@vals);
#for (@scores) {
# print $_,"\n";
# }
my $highest = @scores;
$highest -= 1;
open (FH, ">/home/johann/.spamscore") || die $!;
print FH $scores[$highest];
close(FH);
__END__
Thanks!
--
QUIPd 1.02: (342 of 611)
-> A computer's attention span is as long as its power cord.
http://getafix.homelinux.org/
##2155
--
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]
