On Tuesday, September 9, 2003, at 08:03 AM, LoneWolf wrote:

Ok, this has me bewildered. I have added some fields to a pipe-delimited
file and added the keys to the switch to chomp each line correctly. I am
using the same lines of code that I had before, but now when I run the
script I get 'Use of uninitialized value in concatenation (.) or string at
logs.cgi line 113'. Line 113 is the line marked below.

I've played around with your code a little and here's what I've found. The error is in the print() call, so the answer to your question of what's going on is that on at least one line of your data file, you do not have 12 items. That means that some of the variables are not being filled by the split() and thus are uninitialized when used in the print() string. The two lines you posted are not where the error is. Try running this from the command line to find suspect lines:


perl -ne 'print "Line $. is short.\n" if scalar(split /\|/, $_) < 12' /home/web/data/info/salesa2

I also have some comments about your code. See inline statements below.

the script:
--------------------
sub Ilist
{
use strict;

It's good to see this, but what we've been shown of the code below does not compile under strictures. Also, this is often better placed at the very beginning of your file, outside all sub definitions. Here it is limited in scope and will only apply to the code inside this sub.


$database_file = "/home/web/data/info/salesa2";

This is an example of code that doesn't compile under strict. I assume it's a global variable in your script, but does it really need to be?


open(INF,$database_file) or dienice("Can't open $database_file: $! \n");

Good, but why don't we take advantage of perl's optional parenthesis rules, drop them, and make the line that much easier to read.


@grok = <INF>;

Yuck. ;) You're reading the whole file into memory, but you're really just processing it line by line. Let's just loop over the lines and handle them as they come in.


 close(INF);
 $file3 = "/home/web/sales/inventory_dat.html";

Another global, you really want to try to get away from using globals as much as humanly possible.


open (FIL3, ">$file3") || die "Can't write to $file3 : error $!\n";

You can drop the parenthesis here too if you switch || to or.


print FIL3 "<html><head><title>Inventory List updated on
$date</title></head><body bgcolor=white text=black>\nInventory List updated
on $date<br>\n<table border=1>\n";

$date is another global and the first one I've seen that should probably come from an outside source. You don't need a global for that though, just pass it as a subroutine argument.


 foreach $i (@grok)
 {

This is the loop that belongs above, only as a while loop instead.


  chomp($i);
   ($item_num,$item_desc,$b1,$b2,$b3,$b4,$cc,$vn,$qoh,$qc,$qor,$bc) =
split(/\|/,$i);

Holy cow. :) That's a lot of variables. You don't seem to use them all though. Do we really need them?


#113 here  (print FIL3)
  print FIL3 "<tr><td width=120 align=left>$item_num</td>
                  <td width=330 align=left>$item_desc</td>
                  <td width=75 align=left>$qoh</td>
                  <td width=75 align=left>$qc</td>
                  <td width=75 align=left>$qor</td>
                  <td width=90 align=left>$bc</td></tr>\n";
 }
 print FIL3 "</table></body</html>\n";
}

Let me post a different version of this sub, just to see if I can give you some ideas for tightening it up a little. Yours may not end up exactly like this, because I'll make some assumptions, but maybe it will point you in the right direction:


#!/usr/bin/perl

use strict;
use warnings;

sub items_to_html {
        my($item_file, $html_file, $date) = @_;

        open IN, $item_file or die "File error:  $!\n";
        open OUT, '>', $html_file or die "File error:  $!\n";
        print OUT "<html><head><title>Inventory List updated on $date</title>",
                        "</head><body bgcolor=white text=black>\n",
                        "Inventory List updated on $date<br>\n",
                        "<table border=1>\n";

while (<IN>) {
chomp;
my($item_num, $item_desc, $qoh, $qc, $qor, $bc) = (split /\|/, $_)[0, 1, 8..11];
print OUT "<tr><td width=120 align=left>$item_num</td>",
"<td width=330 align=left>$item_desc</td>",
"<td width=75 align=left>$qoh</td>",
"<td width=75 align=left>$qc</td>",
"<td width=75 align=left>$qor</td>",
"<td width=90 align=left>$bc</td></tr>\n";
}


        print OUT "</table></body</html>\n";
        close IN;
        close OUT;
}

# You might call this sub as follow...

items_to_html( '/home/web/data/info/salesa2',
                        '/home/web/sales/inventory_dat.html',
                        scalar localtime );

__END__

I hope that helps.

James


-- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to