maurkb has asked for the wisdom of the Perl Monks concerning the following question:

I am attempting to search each line of each file in a directory for a specific string, and if found, grab a pertinent piece of data in the same line and then search for all the instances of that data and put them into another file. The files I'm searching are delimited by the pipe ("|") character. I am attempting to use split with the pipe as the delimiter but it seems to return each character into my array instead of each substring between pipes as if the delimiter is undefined. I am relatively new to PERL so I may be missing something quite obvious. My code is below...Any ideas? 
#!/Usr/local/bin/perl;
use Tie::File;
use Getopt::Long;
use warnings;

#string to search for
my($target1) = '6169600047';

#files and directories
my($indir) =  "c:/FAMD_EDI";         
my($outdir) = "c:/FAMD_EDI";
my($outfile) = "FAMD_EDI_Master.txt";

open(OUTDIR, $outdir);                                
open(OUTFILE,">$outdir/$outfile") or die "Cannot create output File";
opendir(MYDIR, $indir) or die "can't open dir $indir: $!";
while (defined($infile = readdir(MYDIR))) 
{
   next if $infile =~ /^\.\.?$/;

   open(FILEHNDL, "$indir/$infile") or die "Cannot open input file $!"
+;
   while ($line = <FILEHNDL>)
   {
       chomp($line);
       #print "line= $line\n";
       @cols = split(/|/, $line);
    
       if ($cols[11] = $target1)
       {
           #complete processing
       }
   }
   close(FILEHNDL);
}
close(OUTFILE);
closedir(MYDIR);

[download]

Replies are listed 'Best First'.
Re: using split
by FunkyMonk (Bishop) on Jun 04, 2007 at 16:10 UTC
    The pipe character is special in a regexp, so you need to escape it:

           @cols = split(/\|/, $line);

    [download]
[reply]
[d/l]
      Ah, I should have realized that. Thanks for the help!
[reply]
Re: using split
by GrandFather (Saint) on Jun 04, 2007 at 21:19 UTC

    Note: it often helps you and us if you reduce your sample code to just demonstrate the bug. Most likely in the process you will solve the problem, but even if you don't you will understand it better. In either case you will remember the lesson better at the end of the day.

    Your sample could have reduced to:

    use strict;
use warnings;

my $line = "This|that";
my @cols = split(/|/, $line);

print "@cols";

    [download]

    Prints:

    T h i s | t h a t

    [download]

    where I expected:

    This that

    [download]

    I know what I mean. Why don't you? has a few other related tips that may help in the future.

    Why no use strict by the way? Especially puzzling as there is a use warnings.

    Added:

    Note too that the 3 parameter open is much preferred over the two parameter open (see open) for lucidity and security.

    if ($cols[11] = $target1)

    [download]

    looks wrong! Do you really intend to assign $target1 to $cols[11]? Note that the if is (effectively) testing the contents of $target1, not equality of $target1 and $cols[11].

    DWIM is Perl's answer to Gödel
[reply]
[d/l]
[select]
      if ($cols[11] = $target1)

      looks wrong!

      Perhaps, but not really. One may need to assign a variable from another expression then test the truthness of that variable. What looks wrong if the variable is assigned to a literal value. This will issue a syntax warning (unless warnings are disabled): 
      my $x;
if ($x = 'a literal value') {
# will issue warnings
}

      [download]
      Note that the if is (effectively) testing the contents of $target1, not equality of $target1 and $cols[11].
      While you are right about the equality part, but the if construct is effectively testing the whole expression, not just the RHS.

      Open source softwares? Share and enjoy. Make profit from them if you can. Yet, share and enjoy!

[reply]
[d/l]
[select]

        I didn't say "is wrong", I said "looks wrong" (yes, I know you didn't say I said that either, but you seem to have reacted as though I did). It looks wrong to the extent that it should either be rewritten to "look right" or be commented. However the only appropriate comment effectively says "I did a dumb thing that needs an even dumber comment so that you know I'm being silly". Much better to rewrite it as:

        $x = $y;
if ($x) {
    ...
}

        [download]

        which makes it quite clear that the assignment is intended and also quite clear what is being tested. Most countries don't tax code by the line so the cost of being clear is pretty low. ;)

        DWIM is Perl's answer to Gödel
[reply]
[d/l]
      I probably should just keep quiet, but... your sample response could have been reduced to: 
      my $line = "This|that";
my @cols = split(/|/, $line);

print "@cols";

      [download]
      While the orthodoxy around here seems to be that everything must use strict and warnings, they're just line noise in code snippets.
[reply]
[d/l]

        I sort of agree in the case of fairly trivial code like this. However the OP didn't use strict so the sample should, and when we say "always use strictures" then don't it weakens the argument a little.

        Note too that this is not a "code snippet", but a full working script that demonstrates the OP's issue. <toungfirmlyincheek>Surely any complete application should include strictures?</toungfirmlyincheek>

        DWIM is Perl's answer to Gödel
[reply]
[d/l]
Re: using split
by lidden (Curate) on Jun 04, 2007 at 22:19 UTC
    When you said
           if ($cols[11] = $target1)

    [download]
    You popably ment
           if ($cols[11] == $target1)

    [download]
    The first one is assigning the value of $target1 to $cols[11] the other checks if they are equal.
    Also in a line like my($indir) =  "c:/FAMD_EDI"; the parens are redundant.
[reply]
[d/l]
[select]
      Well spotted. If I had any ++ left, you'd have got one:)

      The OP should have picked this up himself. I was surprised to see that it generates a warning without strict or warnings:

      #!/usr/bin/perl

my $x;

if ( $x = 4 ) {
    print "Hi";
}
#output
Found = in conditional, should be == at /home/pm line 5.
Hi

      [download]
[reply]
[d/l]
        I was surprised to see that it generates a warning without strict or warnings
        That's one of default warnings, known as mandatory warnings before use warnings; was introduced. These warnings will be enabled by default, but can be controlled with -X switch or warnings. So the following code will issue a warning as you mention: 
        perl -e 'my $x; if ($x = 1) {}'

        [download]
        But this code won't: 
        perl -Xe 'my $x; if ($x = 1) {}'

        [download]
        Neither these snippets: 
        # disable all warnings
no warnings;
my $x;
if ($x = 1) {}

# disable only warnings regarding syntax
no warnings 'syntax';
my $x;
if ($x = 1) {}

        [download]
        References: perllexwarn, perldiag

        Open source softwares? Share and enjoy. Make profit from them if you can. Yet, share and enjoy!

[reply]
[d/l]
[select]
A reply falls below the community's threshold of quality. You may see it by logging in.

Back to Seekers of Perl Wisdom