More loop issues

htmanning has asked for the wisdom of the Perl Monks concerning the following question:

Monks, This should be easy but I'm confused. I'm searching the database for records of jobs that are posted today. Then based on the job type, I do another loop through a different database to find 5 random jobs that match that type. I want to set these to a variable, then print all 5 in an email. I don't know how to create a list of all 5. The code below obviously only prints the first record in the email. How do I stuff the $my_jobs var into a list so I can print all 5 after the second loop is done?

$SQL = "Select * from database where active='yes' AND (TO_DAYS(NOW())-
+TO_DAYS(dateadded) < 1)"; #check for records created today
&Do_SQL;

while ($pointer = $sth->fetchrow_hashref) {
        $jobtype = $pointer->{'jobtype'};
        $toemail = $pointer->{'email'};
        
    
            $SQL3 = "Select * from database where jobtype='$jobtype' A
+ND active='yes' ORDER BY RAND() LIMIT 5";
    
    
            &Do_SQL3;
            while ($pointer3 = $sth3->fetchrow_hashref) {
            $contact= $pointer3->{'contact'};
            $jobnum= $pointer3->{'jobnum'};
            $picture = $pointer3->{'picture'};
            
            
            $my_job = "<tr><td style=\"width:60px;padding:0;\">$pictur
+e - <a href=\"/get-job.pl?jobnum=$jobnum\">$contact</a></td></tr>";

            
    } 
    
}

open (MAILHTML, "|$sendmail $toemail") || die "Can't open $sendmail!\n
+";
print MAILHTML "From: Admin <admin\@admin.com>\n";
print MAILHTML "Reply-to: Admin <admin\@admin.com>\n";
print MAILHTML "To: $toemail\n";
print MAILHTML "Subject: Recommended Jobs\n";
print MAILHTML "Content-Type: text/html\n";
print MAILHTML "$my_job";
close (MAILHTML);
[download]

Comment on More loop issues Download Code

Replies are listed 'Best First'.
Re: More loop issues by morgon (Priest) on Jun 10, 2016 at 23:24 UTC
I have never seen this way to run SQL from Perl (DBI it ain't) but I guess your problem is that `$my_job` gets overwritten in the loop, so you only get the last job mailed. Replace `$my_job = ....` with `$my_job .= ...` (note the dot before the equal sign) to accumulate all found jobs. hth	[reply] [d/l] [select]
Re^2: More loop issues by htmanning (Friar) on Jun 10, 2016 at 23:49 UTC
Oh man, thanks. The dot did it. It works.	[reply]
Re: More loop issues by stevieb (Canon) on Jun 10, 2016 at 23:27 UTC
Before the `while()` loop: `my @my_jobs;` [download] Then change this: `$my_job = "<tr><td style=\"width:60px;padding:0;\">$picture - <a href=\"/get-job.pl?jobnum=$jobnum\">$contact</a></td></tr>";`, to: `push @my_jobs, "<tr><td style=\"width:60px;padding:0;\">$picture - <a +href=\"/get-job.pl?jobnum=$jobnum\">$contact</a></td></tr>";` [download] Then change: `print MAILHTML "$my_job";` to: `print MAILHTML "$_\n" for @my_jobs;` [download]	[reply] [d/l] [select]
Re: More loop issues (more well named subs that take args return values, less global vars, placeholders) by Anonymous Monk on Jun 11, 2016 at 00:02 UTC
You need more subs, less global variables, subs that take arguments and return values, subs with meaningful names, with no numbers in the name When you write programs like this they're easy to read and "loop issues" solve themselves while( $row = $sth->fetchrow_hashref) { my($con, $job, $pic) = GetContactJobPic( $dbh, $row->{jobtype} ); MailNotifyConJobPic( $row->{email}, $con, $job, $pic, ... ); } ... sub GetContactJobPic { my( $dbh, $jobtype ) = @_; my $hashref = $dbh->selectrow_hashref( q{Select * from database where jobtype=? AND active='yes' ORDER BY + RAND() LIMIT 1}m {}, $jobtype ); return @{$hashref}{qw' contact jobnum picture'}; } sub MailNotifyConJobPic { my( $mail, $con, $job, $pic, ... ) = @_; ... ## Mail::Sendmail or ... } [download] Also placeholders are always a good idea bobby-tables.com: A guide to preventing SQL injection in Perl Also because that sendmail stuff is still part of the code you posted, others who read this node should also heed caution	[reply] [d/l]
Re^2: More loop issues (more well named subs that take args return values, less global vars, placeholders) by stevieb (Canon) on Jun 11, 2016 at 01:46 UTC
If we're warning, we should also remind about `use strict;` and `use warnings;` again, and to stop using perl4 type sub calls... The & is not needed.	[reply] [d/l] [select]
Re: More loop issues by $h4X4_\|=73}{ (Monk) on Jun 11, 2016 at 09:20 UTC
you can try `$my_job .= $data."\n";` the dot before the equal adds data at the end of the string. The `."\n"` adds a new line for each new data string. Update: Also add this before you use sendmail for security `$ENV{PATH} = '';`	[reply] [d/l] [select]