Re^7: SQL queries using dynamic array

First, thank you very much for improving your posting style -- it really helps.

I'm looking at your steps, and trying to relate them to the long piece of code you posted -- it's not easy, because the terms (variable names, table column names, etc) that appear in the code do not match the terms you use in describing your two steps.

Apart from that, there is a lot that is wrong with your code as posted. For example, look at how $i is used -- set to zero at the top, then used as a loop counter when cycling through data from a query, then used in while ($i<=2) (it gets incremented inside the loop, but it's probably already greater than 2 before you reach the while loop). Other problems:

You do two separate database connections -- are these two separate databases? If not, you should connect just once.
Inside the while loop, you have "my $rd", and you declare a large subroutine that depends on having $rd in scope -- it would be better to pass $rd as a parameter to the sub, so the sub does not need to be inside the while loop.
There's not nearly enough error checking.

Rather than go further into the problems, let's see if I can rephrase the steps and the code -- you can tell me if I'm guessing correctly about what the task really is:

Get a list of ids for data recipients from the database -- each id will be used to query for the data that needs to be sent, and as part of the email address to send to.
For each id in the list:
1. query the database to get the data for that recipient
2. create an excel file and load it with this person's data
3. send email to the recipient (and attach the excel file?)

I think this is what the code ought to look like -- you'll need check all the details carefully, and make corrections if I've misunderstood anything. Obviously, I can't really test it, but it passes "perl -cw" with no problems.

use strict;
use DBI;
use Spreadsheet::WriteExcel::Big;
use Mail::Sender;

my $dbh = DBI->connect('dbi:Oracle:xxxx','xxxx','xxxx',
               {
               AutoCommit => 0,
               RaiseError => 1,
               PrintError => 1,
               })
    || die "Database connection not made: $DBI::errstr";

my ( $num ) = $dbh->selectrow_array( "select count * from rd_list" );
printf "\nThe database has %d rows in the rd_list table.\n\n", $num;

my $rd_data_sql = 
    "select    week_end_date, SVP, RD, DM, store, wtd_smrr_gain,
         QTD_SMRR_GAIN, wtd_bor_gain, QTD_BOR_GAIN, wtd_cust_gain,
         QTD_CUST_GAIN, WTD_CARD_CLOSED,QTD_AVG_CARD_CL
     from      bonus_4Q_store
     where     SVP <> 'RD' and RD = ?
     order by  svp,dm,store";

my $rd_data_sth = $dbh->prepare( $rd_data_sql );

my $rows = $dbh->selectall_arrayref( "select alignment from rd_list" )
+;

for my $row ( @$rows )
{
    my ( $rd ) = @$row;  # this is the id string (I think)

    print "Working on $rd ...";

    $rd_data_sth->execute( $rd );

    my $rd_data = $rd_data_sth->fetchall_arrayref;

    my $excel_name = make_excel_file( $rd, $rd_data );

    my $result;
    if ( defined( $excel_name )) {
        $result = send_email( $rd, $excel_name );
    }
    else {
        $result = "Failed to write excel file\n";
    }

    print " finished: $result\n\n";
}
exit(0);

sub make_excel_file
{
    my ( $rd, $rd_data ) = @_;
    my $Excelfile = join( "_", "Q4", $rd, "PERFORMANCE RPT.xls" );
    my $excel = Spreadsheet::WriteExcel::Big->new( $Excelfile );

    my $headings = $excel->add_format();
    $headings->set_bold();
    $headings->set_color('white');
    $headings->set_bg_color('blue');
    $headings->set_align('top');
    $headings->set_text_wrap();

    my %ranges = (
    'A1:D1' => "4th Quarter District Manager Quarterly Performance Inc
+entive!",
    'A2:D2' => " ",
    'A3:D3' =>  "Period Measured: 10-01-2008 through 12-31-2008",
    'A4:D4' =>  " ",
    'A5:D5' =>  "Growth:",
    'A6:D6' =>  "plus 18 Customers OR",
    'A7:D7' =>  "plus 27 BOR OR",
    'A8:D8' =>  "plus 3300 in SMRR",
    'A9:D9' =>  " ",
    'A10:D11' =>  "Each store that meets one of the growth goals must 
+also achieve a 5.9% average for Q4.",
    );

    my $worksheet = $excel->addworksheet("Q4_STORE");
    $worksheet->merge_range( $_, $ranges{$_}, $headings )
    for ( sort { length($a)<=>length($b) || $a cmp $b } keys %ranges )
+;

    my %columns = (
    'A:A' => '16.57', 'D:D' => '23.71',
    'F:F' => '16.71', 'G:G' => '16.29', 
    'H:H' => '15.29', 'I:I' => '14.86',
    'J:J' => '16.29', 'K:K' => '15.86',
    'L:L' => '26.86', 'M:M' => '19.00',
    );
    $worksheet->set_column( $_, $columns{$_} ) for ( sort keys %column
+s );

    my $col_headings = $excel->add_format();
    $col_headings->set_bold();
    $col_headings->set_size(10);
    $col_headings->set_color('white');
    $col_headings->set_align('center');
    $col_headings->set_bg_color('blue');
    $col_headings->set_border(2);
    $col_headings->set_shrink();

    my @labels = qw( WEEK_END_DATE  SVP  RD  DM  STORE
                     WTD_SMRR_GAIN   QTD_SMRR_GAIN
                     WTD_BOR_GAIN    QTD_BOR_GAIN
                     WTD_CUST_GAIN   QTD_CUST_GAIN
                     WTD_CARD_CLOSED QTD_AVG_CARD_CL );

    $worksheet->write( 13, $_, $labels[$_], $col_headings ) for ( 0 ..
+ $#labels );

    $worksheet->freeze_panes(14,5);

    my $format = $excel->add_format();
    $format->set_size(10);
    $format->set_align('center');
    $format->set_border(1);
    $format->set_num_format('General;[Red](-General);General');

    $worksheet->write_col( 14, 0, $rd_data, $format ); 
    $excel->close();

    print "$rd DONE WITH Q4 Store DATA! \n";

# there should probably be more error checking above, with a
# 'warn "error message.." and bare "return" (caller gets "undef")
# in the event of a problem...

    return $Excelfile;
}

sub send_email
{
    my ( $recip_name, $excel_name ) = @_;
    my $domain = "rentacenter.com";
    my $to_address = join( "@", $recip_name, $domain );

    my $sender_name = "cory.clay";
    my $from_address = join( "@", $sender_name, $domain );
    print " sending to: $to_address, from $from_address\n";

    my $subject = "Q4 $recip_name PERFORMANCE REPORT";

    my $sender = new Mail::Sender
    {
    smtp => 'xx.xx.x.xx',
    from => $from_address
    };

    $sender->MailFile(
    {
        to => $to_address,
        subject => $subject,
        msg     => "All,
Test Body of email.

Thanks,

Cory Clay
$from_address
\n",
            file    => $excel_name,
    });

    $sender->Close;

# there should probably be error checking above, with a suitable
# error report returned to the caller...

    return "okay";
}
[download]

(updated to put parens around ($num) when assigning the return value from "selectrow_array" at line 14 -- not having parens there would be a mistake, I think.)

Just a few closing remarks about my version vs. yours:

Note that the "main" part of the program is quite brief, and someone reading the code can see pretty easily what it does, and how it relates to the description I gave above -- in fact, the description and the main part of the code are almost the same thing.
Since I removed all the five-line comments from your script (and many of the blank lines around them), more of the actual code is visible on the screen at one time, making it easier to see how variables are used, how the logic flows, etc. Most of your commenting was unnecessary -- it took up a lot of space just to say things that should be obvious from the code itself.
I've tried to keep most of the variable declarations close to where they are used, also making it easier to see the strategy and flow of the code.
I've made the subroutines independent of the main part of the code, passing the parameters that are needed for each sub, making the subs more modular and self-contained (helpful when you want to reuse code, which usually involves moving subs into separate modules/packages).
In the part that sets up the excel file, I've used a hash or array together with a loop instead of repeating a bunch of subroutine calls -- the less repetitive typing you do when coding, the better.

Again, I'm not sure this really does what you want, but I hope it's moving you in the right direction.

Comment on Re^7: SQL queries using dynamic array Select or Download Code