in reply to Re^2: Mail:Audit Resend Problem
in thread Mail:Audit Resend Problem

First, a few code comments:


0) (Update:) Doh--i missed one of the first and foremost things-- add use strict; and use warnings; !!!
1) For this, check perldoc -f localtime:
my ($sec,$min,$hour,$mday,$mon,$year,$wday,$yday,$isdst) = localtime(t +ime); $date = "$mday/$mon/$year $hour:$min:$sec"; # THIS IS WRONG!!!! # do something like this instead: my @time = localtime; my $date = sprintf("%2d/%2d/%4d %02d:%02d:%02d", $time[4] + 1, $time[3 +], $time[5] + 1900, @time[2,1,0]);
2) This (look at perldoc -f exists):
while(my ($key, $value) = each(%forward)) { if ($to =~ $key) { $local_address = $key; $forward_address = $value; } }
should be replaced with:
if( exists $forward{$to} ){ $local_address = $key; $forward_address = $value; }
3) Use (see DBI docs) heredocs and placeholders. (Also, i believe this 'LEFT JOIN' should be just 'JOIN')
$sql = <<EOF; SELECT "tblForwardMail"."ForwardMailID", "tblFilterMail"."FilterMailEm +ail" FROM "tblForwardMail" LEFT JOIN "tblFilterMail" ON "tblForwardMail"."ForwardMail +ID"="tblFilterMail"."ForwardMailID" WHERE "tblForwardMail"."ForwardMailTo"=? EOF # $sth = $dbh->prepare($sql); # $sth->execute( $local_address ); # while (@row = $sth->fetchrow) { # push(@filter, $row[1]); # } # # But it seems like what you want next is just: (undef,$filter_address) = $dbh->selectrow_array($dbh,{},$local_address +); # make sure to pre-declare $filter_address
4) This:
foreach my $email_address(@filter) { if (($from =~ $email_address)){ $store = "YES"; } }
could be:
$store = "YES" if grep( $_ eq $from, @filter );
but if you use my comment #3, then it's just:
$store = "YES" if $from eq $filter_address;
BUT, you should treat $store as a boolean and give it true (1) and false (0, undef) values, so you don't have to string match 'YES' later..
$store = 1; # true $store = 0; # false if( $store ){ ... } if( ! $store ){ ... }
5) This:
#Declare variables to be used including command line variables if need +ed my ($stored_email, $log, $to, $from, $forward_address, $store, $body, +$sql, $sth, $date, $local_address);
is "c-ish" .. here it's usally better to declare stuff just when it's being used (especially if it only exists in a limited block/scope)

gotta cut back to work for a moment ;) but then i'll go back and see if can find the bug..

Replies are listed 'Best First'.
Re^4: Mail:Audit Resend Problem
by tertullian01 (Acolyte) on Jun 21, 2005 at 17:23 UTC
    Thank you for your comments. I will be the first to admit my Perl is really rusty since I have moved from a system admin role to primarily PHP web development. But on with the comments. I have taken some of you comments to heart and implemented them including (corresponding numbers to your comments):
    0)use warnings;
    1)Proper use of localtime and sprintf
    3)Fixing the sql statements
    4)Making store boolean rather than string
    My comments on your other suggestions follow:
    2)The reason I am using the while statement rather than just using exists is because I have found the address that I pull out of the email can be the pure email address (test@test.com) or the email address with a name ("TEST USER" <test@test.com>). Using the while statement allows me to use an approximate match.
    3)I am not using the more simplified selectrow_array that you suggested because there could be many addresses to filter and so those need to be stored in an array
    4)Although I using boolean rather than string I have kept the foreach in there fore the same reason as number 2 above. I need to do an approximate match rather than an exact match.

    I think part of the problem you are having in diagnosing is an incomplete description on my part. We are setting up a forwarding server where people can have an email address on our domain that forwards to their own personal email address. What we are hoping is people that have ocntacts around the world will sign up and allow us to harvest the body of their email to look at linguistic problems so as a result the setup is this. Each email address in our domain will forward to one personal email address and according to the user's contacts and what they submit to us each email will be stripped and stored for further linguistic analysis. I hope that clears it up more.