Beefy Boxes and Bandwidth Generously Provided by pair Networks
There's more than one way to do things
 
PerlMonks  

Re^4: Search and Remove

by PyrexKidd (Monk)
on Jun 16, 2010 at 01:09 UTC ( #844953=note: print w/ replies, xml ) Need Help??


in reply to Re^3: Search and Remove
in thread Search and Remove

So how about this?

#!/usr/bin/perl use strict; use warnings; use diagnostics; if (@ARGV = 2){ my %delList; my @orgList; open (BEEPON, >>, "/path/to/beepon.file); open (BEEPOFF, >>, /path/to/beepoff.file); open (DELLIST, <, $ARGV[0]); foreach (<DELLIST>){ %delList{chomp($_)} = 1; } close DELLIST; open (ORGLIST, <, $ARGV[1]); @orgList = <ORGLIST> close (ORGLIST); for (my $i = 0; $i < @orgList; $i++){ if (exists $del{$orgList[$i]}){ print BEEPON $orgList[$i]; } else print BEEPOFF $orgList[$i]; } } close BEEPON; close BEEPOFF;
I can see how this would be more efficient. I'm not really sure that it is what you outlined... I'm still not entirely understanding the use of hashes. ie why are all the values set to 1?


Comment on Re^4: Search and Remove
Download Code
Re^5: Search and Remove
by rjt (Deacon) on Jun 17, 2010 at 10:02 UTC

    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.

      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?

      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.

        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...

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://844953]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others having an uproarious good time at the Monastery: (6)
As of 2014-10-22 00:56 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    For retirement, I am banking on:










    Results (112 votes), past polls