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

I've been banging my head against a wall trying to get my code to work for a couple of days now and am giving up to ask for help. I have a directory of logs (pipe delimited) that I am searching for a phone number and returning data if found. I'm using an input file for the search. That part works, but I also want to return a message if a phone number in the input file is not found. It seems simple enough but if a number is not found I get the message for each line of data in each file. I only want one message if the number is not found in any of the files. I think it's a problem with where I'm putting the code in the loop, but I can't figure out where the right spot is.

Here is the code without the part to return a message if a number is not found:

use strict; #use warnings; my $record; my $tele; my $esn; my $coid; my $error; my $comptn; sub search_tn; my $directory = "C:\\TEMP\\files"; opendir( DIR, $directory ) || die "Unable to open directory - $!\n"; my @files = grep /^R/, readdir( DIR ); closedir( DIR ); open( OUTFILE, "> $directory\\search_results.txt" ) || die "Unable to open write file! - $!\n"; open (COMPFILE, "c:\\temp\\files\\search tns.txt") || die "Unable to open search file! $!\n"; while (<COMPFILE>) { $comptn = substr $_, 0,10; search_tn($comptn); } sub search_tn { foreach my $file (@files) { open( FH, "$directory\\$file" ) || die "Unable to open $file - $!\n"; while( <FH> ) { my @record = split /\|/, $_; next until $. > 1; if ($record[0] eq "TLR") { } else { $tele = substr $record[2], 3, 10; if ($comptn == $tele) { print OUTFILE "====TN Found====\n"; print OUTFILE "FILE: $file\n"; print OUTFILE "TN: $tele\n"; $esn = substr $record[3], 3,5; print OUTFILE "ESN: $esn\n"; $coid = substr $record[4], 3,5; print OUTFILE "COID: $coid\n"; $error = substr $record[9], 3,3; if ($error == "") { print OUTFILE "Error: no error \n\n"; } else { print OUTFILE "Error: $error \n"; print OUTFILE "================\n\n"; } } } } close (FH); } }

Here is a variation of the code I was using to return a message if the number is not found.

if ($comptn != $tele) { print OUTFILE "TN $comptn not found \n"; }

as a side note I'm pretty sure my code is not very efficient, if you have suggestions on how I can better write any code feel free to give me suggestions. Thanks!

Replies are listed 'Best First'.
Re: Need Help With Loops
by jwkrahn (Abbot) on Sep 23, 2009 at 17:45 UTC
    use strict; #use warnings;

    That should be:

    use strict; use warnings;

    while (<COMPFILE>) { $comptn = substr $_, 0,10; search_tn($comptn); } sub search_tn {

    That should probably be:

    while (<COMPFILE>) { search_tn( substr $_, 0, 10 ); } sub search_tn { my $comptn = shift;

    next until $. > 1;

    There is no need for a loop there!

    next if $. == 1;

    if ($record[0] eq "TLR") { } else {

    Better as:

    next if $record[0] eq "TLR";

    if ($error == "") {

    You are using a numerical comparison on a string which will not work correctly.

    if ($error eq "") {

Re: Need Help With Loops
by GrandFather (Saint) on Sep 23, 2009 at 21:20 UTC

    "I've been banging my head against a wall" and #use warnings; often go well together! Why did you turn off warnings?

    if ($error == "") {

    generates the warning "Argument "" isn't numeric in numeric eq (==) at ..." which would have alerted you to at least one coding error.

    Oh, and I should have mentioned in reply to either of your previous posts that you should use the three parameter version of open with lexical variables for the file handle:

    my $filename = "$directory\\search_results.txt"; open my $Outfile, '>', $filename or die "Can't open for writing $filen +ame - $!\n";

    Generally if you need to repeated lookups the answer is to use a hash. The following (untested) code first populates a hash from the input files, then runs over the search file and generates the report file:

    use strict; use warnings; my $directory = "C:\\TEMP\\files"; my %numbers; opendir my $dirScan, $directory or die "Unable to open $directory - $! +\n"; while (my $file = readdir $dirScan) { next if !-f $file or $file !~ /^R/; open my $inFile, '<', "$directory\\$file" or die "Unable to open $file - $!\n"; <$inFile>; # Skip the header line while (<$inFile>) { my @record = split /\|/, $_; next if $record[0] eq "TLR"; my $tele = substr $record[2], 3, 10; my $esn = substr $record[3], 3, 5; my $coid = substr $record[4], 3, 5; my $error = substr $record[9], 3, 3; $numbers{$tele} = {file => $file, esn => $esn, coid => $coid, error => $erro +r}; } close $inFile; } closedir $dirScan; my $filename = "$directory\\search_results.txt"; open my $outFile, '>', $filename or die "Unable to open write $filename! - $!\n"; open my $cmpFile, '<', "c:\\temp\\files\\search tns.txt" or die "Unable to open search file! $!\n"; while (<$cmpFile>) { my $target = substr $_, 0, 10; if (exists $numbers{$target}) { my %record = %{$numbers{$target}}; my ($file, $esn, $coid, $error) = @record{qw(file esn coid err +or)}; print $outFile "====TN Found====\n"; print $outFile "FILE: $file\n"; print $outFile "TN: $target\n"; print $outFile "ESN: $esn\n"; print $outFile "COID: $coid\n"; if (! $error) { print $outFile "Error: no error \n\n"; } else { print $outFile "Error: $error \n"; print $outFile "================\n\n"; } } else { print $outFile "Missing: $target\n"; } } close $cmpFile; close $outFile or die "Trouble closing the report file: $!\n";

    True laziness is hard work
Re: Need Help With Loops
by kennethk (Abbot) on Sep 23, 2009 at 17:16 UTC
    You need to consider the logic of your situation. When exactly do you want to announce that there wasn't a match? Since the answer is "I also want to return a message if a phone number in the input file is not found", then clearly you should only perform output after you have reviewed the entire input file. That should give you an idea of where in your code the output should go - combine that with the idea of a $found variable that gets changed when you find what you are looking for, and you have your solution.