What am I doing wrong in my Perl script written to parse a CSV file?

Question

I have two scripts in which I'm experimenting with CSV_XS. In the first, I hard-coded everything: source directory, filename, and the csv delimiter I wanted to look for. The script works great. In the second, however, I try to dynamically discover as much as possible. That script seems to run, but it outputs nothing.

I'm having trouble figuring out why, and I was hoping you fine Perl folks wouldn't mind lending a second set of eyes to the problem:

First, the successful script:

#!/usr/bin/perl -w
use Text::CSV_XS;
my @records;
my $file = 'Data/space.txt';
my $csv=Text::CSV_XS->new({ sep_char => " " });

open(FILE,$file) || die "Couldn't open $file: $!\n";
while (<FILE>){
 $csv->parse($_);
 push(@records,[$csv->fields]);
}
close FILE;

foreach (@records){
 print $_->[0], ",", $_->[1], ",", $_->[2], ",", $_->[3], ",", $_->[4], "\n";
}

And second, the "failing" script:

#!/usr/bin/perl -w
use Text::CSV_XS;

$input_dir = $ARGV[0]; #I pass "Data" on the command line
my @records;

opendir(DIR, $input_dir) || die "cannot open dir $input_dir: $!";
my @filelist = grep {$_ ne '.' && $_ ne '..'} readdir DIR;
closedir DIR;

foreach $file (@filelist){
 print "Input file='",$input_dir,"/",$file,"'\n";
 if ($file =~ /comma/) {$sep=','}
    elsif ($file =~ /pipe/) {$sep='|'}
    elsif ($file =~ /space/) {$sep=' '}
    else {die "Cannot identify separator in $file: $!";}
 print "Delimiter='",$sep,"'\n";   
 open(FILE,$input_dir||"/"||$file) || die "Couldn't open $file: $!\n";
 my $csv=Text::CSV_XS->new({ sep_char => $sep });
 while (<FILE>){
  $csv->parse( $_ );
     push(@records,[$csv->fields]);
  print "File Input Line:'", $_ ,$csv->fields,"'\n";
 };
 close FILE;
}

foreach $record (@records){
 print $record->[0], ",", $record->[1], ",", $record->[2], ",", $record->[3], ",", $record->[4], "\n";
}

Answer 1

This line looks kind of suspect:

open(FILE,$input_dir||"/"||$file) || die "Couldn't open $file: $!\n";

I don't think you want to put those || in there. What that does is check to see if $input_dir is true, then if it isn't, it check to see if "/" is true (which it always is). Your $input_dir is likely always true, so you're just opening the $input_dir.

You should be using File::Spec to create your fully-qualified files:

my $fullfile = File::Spec->catfile( $input_dir, $file );
open( FILE, $fullfile ) || die "Couldn't open $fullfile: $!\n";

This will "do the right thing" in putting a / where appropriate (or, if you're on Windows, \). Then pass that in to your open() command.

Further, you should be using lexical filehandles and directory handles, along with the three-option open():

open my $fh, '<', $fullfile or die "Could not open file $fullfile: $!\n";

Lexical filehandles are much safer, as they can't get overridden by some other module defining a FILE filehandle. Three-option open() is easier to understand and isn't prone to error when you have a filename that has a > or < or | in it.

If you want to get really crazy, put use autodie; at the top, so you don't even have to check for the return value of open() or opendir():

use autodie;
open my $fh, '<', $fullfile;