Thanks, Charles, for your suggestions. I think I've incorporated most of them.
Your regex is a lot more practical than what I was using.
Here is the improved script. I can live with the one warning
"Newline in left-justified string for printf at ./describe_skus.pl line 29".
John

#!/usr/bin/perl -w
use strict;
use DBI;

# purpose: To take sku numbers from a text file, "no-imgs.txt",
# and print out their database descriptions, to a separate text file, "need-images.txt".

open INFILE, "no-imgs.txt" or die "Input file failed to open: $!";
chomp( my @skus = map /^\s*(\d*)\s*$/, <INFILE> );
close INFILE;

open OUTFILE, "> need-images.txt" or die "Output filehandle not open: $!";
print OUTFILE"SKU \t TITLE \t\t\t\t VENDOR_STOCK_NO \t VENDOR_ID \n";
print OUTFILE" - - - - - - - - - - - - - - - - - - - - - - - - - - -\n";

my $dbh=DBI->connect('dbi:mysql:productdb','script','') ||
die "Error connecting to database: $DBI::errstr";

my $query2 = q{select sku, title, vendor_stock_no, vendor_id
FROM products
WHERE sku=?
};

for my $sku (@skus) {
my $sth2= $dbh->prepare($query2) || die "Prepare failed: $DBI::errstr";
$sth2->execute ($sku) || die "Couldn't execute query: $DBI::errstr";

while ( my $columns = $sth2->fetchrow_arrayref() ) {
printf OUTFILE ("%-4d %-40s %-22s %d","@$columns[0]","@$columns[1]","@$columns[2]","@$columns[3]");
print OUTFILE "\n";
}
$sth2->finish ();
}

$dbh->disconnect || die "Failed to disconnect\n";
close OUTFILE or die "Cannot close output file: $!";


Charles K. Clarkson wrote:

john <mailto:[EMAIL PROTECTED]> wrote:

: Yes, you're right. I changed this line:
:     while ((my $ref) = $sth2 ->fetchrow_hashref("$num")) {...
: by removing the parameter, and quotes around 'my $ref' :
:     while (my $ref = $sth2 ->fetchrow_hashref()) {...
: and the script works now.

   For the archives: Those were parenthesis, not quotes, which
were replaced around "my $ref".


John,

   I'm glad you solved this problem, but there are some other
items in your code that need mentioning. To save some line
wrapping I'm going to delete two levels of email indentation. I'm
also going to leave the new lines off the end of my "die"
statements. This gives a better idea of where problems originated.
Read the perl docs for "die" for details on why.


: open INFILE, "no-imgs.txt" or die "Input file failed to open: $!\n";
: open OUTFILE, "> need-images.txt" or die "Output filehandle not open:
$!\n";
: my @nums = <INFILE>;

   This is the first section that I wanted to comment on. When
programming, we are writing a story. We often introduce some
characters at the beginning, describe the setting, and bring in
more characters as the story unfolds.

        When writing a script it becomes important to keep the reader
in mind. Program readers are the people who come behind: code
maintainers and such. I often program for the only one maintainer,
my future self. I know that in six months or a year, I will not be
the same programmer I am now and that some of the things I do may
look very odd. I try to keep my utterly confused future self in
mind while I write.

        I mention this because I want other script authors to grasp
the importance of the timeline in a story. In perl, we generally
wait to the last moment to bring out new characters, like lexical
variables. Sometimes we want to introduce a character and then
remove them as soon as possible, like I/O operations.

        So, several things struck me as odd about these there lines.
First, we don't close INFILE. We're finished with it, why leave it
open? Second, why open a file, do something unrelated (though
similar) and then suck in the open file? Why not open the file,
suck in the file, and close the file then move to the next task?
Third, is $nums really a descriptive name for a variable? How
about @skus, @sku_numbers, or @stock_keeping_unts.

        Fourth, why do only this very limited processing of the file?
Later on, we strip line endings and white space to end up with
just sku numbers. Why not just start with those? The approach used
in the script to obtain sku numbers was exclusionary. Exclude the
characters we don't want, the rest are sku numbers.

        In a thread on email regular expressions last month, one guru
mentioned that a safer regular expression for that subject was
one in which we only include valid characters for a field. Let's
do the same for these sku numbers.

   I don't know how your sku numbers are formatted, so I'll just
make a format up. You can modify that to meet your needs. I'll use
a simple 6 digit number, like 123456 or 024569.

   (I use the three argument open and tend to recycle file
handles. Unless it is already open I just keep using $fh for every
file I open.)

my $file = 'no-imgs.txt';
open my $fh, '<', $file or die qq(Cannot open "$file": $!);
chomp( my @sku_numbers = map /^\s*(\d{6})\s*$/, <$fh> );
close $fh;

        Wow, that's a mouthful!
        
        You can chomp an array all at once.
        
chomp( @sku_numbers );

        You can chomp an array as it is assigned.
        
chomp( my @sku_numbers = <$fh> );

        And since "map" is just another assignment, you can chomp an
array while you map it into a nicer form.
        
chomp( my @sku_numbers = map /blah/, <$fh> );


        The regex can be broken down as follows.

        /                       # open match - same as m/
                ^                       # Anchor this match to the beginning
of the line.
                \s*                     # match 0 or more white space
characters.
                (                       # begin capture.
\d{6} # capture six digits in a row. ) # end capture.
                \s*                     # match 0 or more white space
characters.
                $                       # Anchor this match to the end of
the line.
        /                       # close match

        If there is no match, map moves on to the next line. The
result is only valid sku numbers remain @sku_numbers.


: my $dbh=DBI->connect('dbi:mysql:productdb','script','') ||
:   die "Error connecting to database: $DBI::errstr\n";
: : my $query2 = qq{SELECT `sku`, `title`, `vendor_stock_no`,
: `vendor_id` FROM `products` WHERE `sku`=?};

   I assume the back ticks are needed in the mySQL admin script
you use, but they are not required by the DBI (AFAIK).

my $query2 = qq{SELECT sku, title, vendor_stock_no, vendor_id
                                                                        FROM
products WHERE sku=?};
                                                                        
        I'm still experimenting with styles, but I find a little more
white space handy when reading and writing queries. The extra
spacing makes it real easy to spot the column names.

my $query2 = q{
        SELECT sku, title, vendor_stock_no, vendor_id
        FROM products
        WHERE sku = ?
};

# Or:

my $query2 = q{
        SELECT
                sku,
                title,
                vendor_stock_no,
                vendor_id

        FROM
                products

        WHERE
                sku = ?
};


: for my $num (@nums) {
:    last unless $num; #exit at end-of-input-file

   After slurping a file into an array, the end of the file
becomes the end of the array. No need to check it here.


:    next if $num =~ /^\s*\t*\n$/;  # to skip over blank lines
:    $num =~ s/^\s+//; # delete leading spaces
:    chomp $num;

   With all the preprocessing we did above we can simply use this
now.
foreach my $sku_numeber ( @sku_numbers ) {


:    my $sth2= $dbh->prepare($query2) || die "Prepare failed:
$DBI::errstr\n";
:       $sth2->execute ("$num") ||
:                                die "Couldn't execute query:
$DBI::errstr\n";

        Why is $num in quotes? $query2 isn't? Do not quote variables
unnecessarily. See perlfaq4 for details on why.


:    while ((my $ref) = $sth2 ->fetchrow_hashref("$num")) {
:        print OUTFILE "sku, \t title, \t vendor_stock_no, \t vendor_id \n",

   I have never gotten an answer on this one. Why place a space
at the *end* of a line? Every person I call on this leaves me
hanging. It doesn't affect your program at all. Feel free to
ignore me, also. :)


:             $ref->{sku}, $ref->{title}, $ref->{vendor_stock_no},
:    $ref->{vendor_id}; }

        Why use fetchrow_hashref() if you aren't doing anything to the
values. fetchrow_arrayref() makes more sense.

$file = 'need-images.txt';
open $fh, '>', $file or die qq(Cannot open "$file": $!);

while ( my $columns = $sth2->fetchrow_arrayref() ) {
        print OUTFILE
                "sku, \t title, \t vendor_stock_no, \t vendor_id\n",
@$columns;
}

close $fh;



HTH,

Charles K. Clarkson

--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
<http://learn.perl.org/> <http://learn.perl.org/first-response>


Reply via email to