http://www.perlmonks.org?node_id=737573


in reply to a problem with a point...

Bass-Fighter:

I'm in a verbose mood today, so please forgive the length of this message. While reading through your code, I thought I'd offer a few random suggestions:

First, I notice that you're using the construct:

my ($rowref, $ff, @bigmail); ... snip ... while($rowref= $st->fetchrow_hashref){ $bigmail[$ff]= $rowref->{'email'}; $ff++; }

You're using the variable $ff only to help you load the @bigmail array, so you're trying to keep $ff synchronized with the index of the next slot in @bigmail to fill. Perl gives you the equivalent $ff for free: Using @bigarray in scalar context evaluates to the number of items in the array--and since perl numbers array slots from 0 .. N-1, the number of items in the array is the index of the next available slot: N. So you can eliminate $ff like so:

my ($rowref, @bigmail); ... snip ... while($rowref= $st->fetchrow_hashref){ $bigmail[@bigmail]= $rowref->{'email'}; }

And since you're just adding an item to an array, you could use push or shift to add the item to the array. (One adds the item to the end of the array, and one inserts it at the beginning of the array. If you don't care about order, it doesn't matter which one you use.) So the statement:

$bigmail[@bigmail]= $rowref->{'email'};

could also be expressed as:

push @bigmail, $rowref->{'email'};

Next, regular expressions incur some overhead. Using them to match a single constant value makes perl compile a regular expression and then execute it for each string. If you change this:

if ($adres=~ /<$ecode>/){

to this:

if ($adres eq "<$ecode>"){

you can avoid that little bit of overhead. I wouldn't go to the trouble of changing it, but I thought I should mention it to you.

There are a couple of things about the following snippet I should mention:

my $zz; my $fg = @bigmail; my $pl=0; my $ecode; for (my $i=0; $i<$fg; $i++){ $ecode= $bigmail[$pl]; $pl++; if ($adres eq "<$ecode>"){ $zz=$ecode; } }

First, you're introducing a new variable ($fg) to hold the number of items in your array. But you could just use @bigmail instead, and your code would be a bit easier to read because I wouldn't have to figure out what the variable $fg contains. Making that change gives you:

my $zz; my $pl=0; my $ecode; for (my $i=0; $i<@bigmail; $i++){ $ecode= $bigmail[$pl]; $pl++; if ($adres eq "<$ecode>"){ $zz=$ecode; } }

Next, the variable $pl isn't necessary, as your loop variable $i is serving the same purpose: stepping through the available item indexes in @bigmail. So you can remove it, giving you:

my $zz; my $ecode; for (my $i=0; $i<@bigmail; $i++){ $ecode= $bigmail[$i]; if ($adres eq "<$ecode>"){ $zz=$ecode; } }

But don't do it this way, either. Perl gives you a nicer version of the for statement--it lets you use the items in the array directly, rather than having to use an artificial variable. So you can write that loop using the nicer for statement like this:

my $zz; for my $ecode (@bigmail){ if ($adres eq "<$ecode>"){ $zz=$ecode; } }

I could go on with grep to simplify that loop further, but I'll leave that as an exercise to the reader. Instead, I'll just mention that the variable name $zz isn't very informative. You can make your code easier to understand by using more descriptive variable names.

Ah, well ... off to work! I hope you find something in here you can use.

...roboticus