Re^5: Search and Remove

[rjt]

You're getting there. Nice to see you coming back with improving iterations. Comments by section, below:

First of all...

I can tell by looking that your code won't work--you're interchanging %delList and %del, for example. Make it easier on us, please: test your own programs.

That being said, there is still plenty I can tell you about your code.

The DELLIST loop

You deviated from my example in several important aspects:

I'm not sure why you've gone with the bareword filehandle approach. Best practice (as in my example) is to go with a lexical filehandle with my.

Second: Hash and array members are always scalars, so the sigil is always $ in Perl 5 (unless you're working with slices, but that doesn't apply here.) So your %delList{...} should be $delList{...}, to access individual hash elements. Your @array usage appears to be correct.

Third: Re-read chomp carefully. You're using its return as a hash key value, but it returns "the total number of characters removed from all its arguments", which is not what you want. You want the chomp'd string. That's why I did chomp first and then stuck $_ into the hash.

Last, to answer your question about hashes and why all of the values are set to 1, first let me make sure you're not talking about keys. With your code as-is, probably every key will be set to 1, because that's what chomp will return for each line (each line has one newline). With me so far?

Hash keys are the strings used to address data elements in the hash. Hash values are the scalars containing whatever data you want to be associated with each key. For example:

my %age; # Age of a person, in years
$age{'Sonny'}  = 30;
$age{'Junior'} = 10;
$age{'Poppa'}  = 40;

foreach my $person (sort keys %age) {
    print "$person is $age{$person} years old\n";
}

In your case, we want to know if a certain entry should be written to one file or another. The way we define that is, if a key exists in %dellist for that entry, that entry should be written to "beepon", else it's written to "beepoff". (What these names mean, I have no idea, by the way.) We assign 'something' a true value in %dellist with $dellist{'something'} = 1. In the loop, that 'something' happens to be the line that was read from the $dellist filehandle, minus the trailing newline.

Efficiency of scanning a list (your old method) versus using a hash is something similar to opening every drawer in your chest of drawers one by one until you find one that has socks, versus remembering that $drawer{'socks'} is 'topleft' and just opening the top left drawer. Like most analogies, it's not perfect, but describes the efficiency gain fairly well.

foreach

Your for loop looks mostly functional, but you don't need to use C-style constructs, and you certainly don't need to read the entire file into an array first (waste of memory, possibly significant). You also don't need a clumsy if/else; there's a fabulous example of how to specify a filehandle as an expression right in print.

open my $orglist, '<', $ARGV[1] or die "Can't open $ARGV[1]: $!";
foreach my $item (<$orglist>) {
    chomp($item);
    print { exists $dellist{$item} ? $beepon : $beepoff } $item . "\n"
+;
}
close $orglist;

Of course, the above assumes you've already done away with the bareword filehandles in favour of scalars.

[PyrexKidd]

Ok so here is my next attempt... but I'm having trouble.

#!/usr/bin/perl 
use strict; 
use warnings; 
#use diagnostics; 

if (@ARGV = 2){ 
    my %delList;  
    open (my $BEEPON, >>, "./beepon.file"); 
    open (my $BEEPOFF, >>, "./beepoff.file"); 
    open (my $DELLIST, <, $ARGV[0]); 
    
    foreach (<$DELLIST>){ 
        chomp($_);
        $delList{$_} = 1; 
    } 
    close $DELLIST; 
    open (my $ORGLIST, <, $ARGV[1]); 
    
    foreach my $entry (<$ORGLIST>){ 
        chomp($entry);
        print { exists $delList{$entry} ? $BEEPON : $BEEPOFF } $entry 
+. "\n";
    }
close $BEEPON; 
close $BEEPOFF; 
}

here are the errors I get when running the code:

readline() on closed filehandle $DELLIST at ./delete_list_ver3.pl line
+ 12.
Use of uninitialized value $ARGV[1] in concatenation (.) or string at 
+./delete_list_ver3.pl line 17.
readline() on closed filehandle $ORGLIST at ./delete_list_ver3.pl line
+ 19.
syntax error at ./delete_list_ver3.pl line 8, near ", >>"
syntax error at ./delete_list_ver3.pl line 9, near ", >>"
Unterminated <> operator at ./delete_list_ver3.pl line 10.

...

grrr...

how can a file I just opened be closed?

[PyrexKidd]

ok so I reread your post. I made some changes to the way I open the file handles. I'm still getting one error.

#!/usr/bin/perl 
use strict; 
use warnings; 
#use diagnostics; 

if (@ARGV = 2){ 
    my %delList;  
    open my $BEEPON, '>>', "./beepon.file" || die "Can't open beepon.f
+ile $!"; 
    open my $BEEPOFF, '>>', "./beepoff.file" || die "Can't open beepof
+f.file $!"; 
    open my $DELLIST, '<', $ARGV[0] || die "Can't open $ARGV[0] $!"; 
    
    foreach (<$DELLIST>){ 
        chomp($_);
        $delList{$_} = 1; 
    } 
    close $DELLIST; 
    open my $ORGLIST, '<' , $ARGV[1] || die "Cant open $ARGV[1] $!"; 
    
    foreach my $entry (<$ORGLIST>){ 
        chomp($entry);
        print { exists $delList{$entry} ? $BEEPON : $BEEPOFF } $entry 
+. "\n";
    }
close $BEEPON; 
close $BEEPOFF; 
}

here are the resulting errors:

$./delete_list_ver3.pl test.list test_master.1

readline() on closed filehandle $DELLIST at ./delete_list_ver3.pl line
+ 12.
Use of uninitialized value $ARGV[1] in concatenation (.) or string at 
+./delete_list_ver3.pl line 17.
Cant open  Bad file descriptor at ./delete_list_ver3.pl line 17.

WTF???

as far as the BeepOn Vs BeepOff are concerned... Not very descriptive taken out of context are they.

this is part of a bigger project that generates a config file for an asterisk conference bridge. I often find it necessary to move ID's from one list to the other, and this is the result.

honestly the boss is happy from the first iteration of this code; I'm not because I am interested in learning Perl, and learning how to use Perl effectively and efficiently, and this seems like a good learning project, (at least I've learned alot I think) and progressing my perl abilities will help me not only professionally but academically as well.

For the record I really appreciate your help all along the way.

[PyrexKidd]

HAHAHAHAHAH!!!!!

You ever have an issue and it's as simple as a missing `=` ?

This line: if (@ARGV = 2){

Should be: if (@ARGV == 2){ Talk about a n00b mistake. Grrr...