Beefy Boxes and Bandwidth Generously Provided by pair Networks
Don't ask to ask, just ask
 
PerlMonks  

A few simplifications [Re: a problem with a point...]

by roboticus (Canon)
on Jan 20, 2009 at 14:01 UTC ( #737573=note: print w/ replies, xml ) Need Help??


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


Comment on A few simplifications [Re: a problem with a point...]
Select or Download Code

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://737573]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others wandering the Monastery: (13)
As of 2014-07-31 22:07 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    My favorite superfluous repetitious redundant duplicative phrase is:









    Results (254 votes), past polls