> #!/usr/bin/perl
> #
> # cur2csv.pl
> #
> 
> use strict;
> use vars qw($started);
> use vars qw($cur_sym $cur_desc $usd_unit $units_usd);
> use vars qw($year $month $mday $hour $minute $second $timezone);
> use vars qw($conv_date $date $time $tz);

Instead of the battery of use vars, I suggest using my.
As in:

    my ($year, $month, $mday, $hour, $minute, $second, $timezone) =

etc.

A lot less typing and clutter.

> use Date::Manip;
> use String::Strip;
> 
> use DBI;
> use DBD::Pg;

You don't seem to be using functions from most of these
packages, so consider taking them out. You can always
comment some out to start with and see if you get errors,
as in:

# use DBI;
# use DBD::Pg;

> open (OUTFILE, ">", "currency.csv") || die "Can not open currency.csv
> for writing";

You should get in to the habit of including $!
(or $ERRNO, or even $OS_ERROR, if you
prefer one of these longhand aliases), in die
strings for die's following system calls. $!
contains the OS error of the last system call.
Most people just dump it on the end, as in:

    open OUTFILE, ">currency.csv"
        or die "Can not open currency.csv for writing: $!";

> while (<INFILE>) {
> 
>     ($cur_sym, $cur_desc, $usd_unit, $units_usd) =
>     /^([A-Z]{3})\s+([A-Za-z\s]{28})\s+(\d+\.\d+)\s+(\d+\.\d+)/;
> 
>     printf OUTFILE "%s\,%s\,%s\,%s\,%s\,%s\,%s\n",
> $date, $time, $tz, $cur_sym, $cur_desc, $usd_unit, $units_usd;

You've put this print line ahead of the tests of whether the
line matched. So it will print out even for lines that aren't
currency lines, and so aren't matching, and so $cur_sym
etc. are empty.

>     $cur_sym and $started++;
> 
>     not $cur_sym and ($started and last) or next;
> 
>     $started or print STDERR "Didn't find a currency line";

What's this doing here?!? This has to wait until we've done
the loop. Basically, if we got through the entire loop but we
never $started processing currency lines, we know we have
a problem. But that has to wait until we're through the loop.

> #     substr($cur_desc, 0, 1) = '';  # Delete leading space

This should go, given how you changed the regex not to
match a space at the end of $cur_sym.

Here is where the print statement belongs.
> 
> } 

Here is where the 
>     $started or print STDERR "Didn't find a currency line";

belongs.

> close(INFILE);
> close(OUTFILE);
> print STDERR "\n";
> 
> 1;

hth.

Reply via email to