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

This is sucking the life out of me. I've got a big, text file. I go through it, and extract the info i want and stuff it in a hash. There's actually two sets of data though, so I essentially go through the same file twice (it's tied to an array, so this is easy). I specifically
Loop through
Store first set of data in a hash.
Print key-value pairs.
Delete everything in the hash
Repeat.

Most keys share the same label. The values are integers. I even print out the value (and have tried setting them to 0) after deleting to make sure and it looks like they're deleted. However, when I print set B, with the supposedly only set b's data the values are always the sum of the two. (I've confirmed by reversing the order: gathering and printing set b first, then set a. Set b is then "correct" and set a has the sum of the two values for each key).

Below is the subroutine I use to clean out the hash.
sub clean_hash {
        while ( ( my $key, my $value ) = each %hash ) {               
+               
        delete $hash{$key};
        }
}

[download]
My other option would be to use dynamic variable names for hashes, but I've read many places that this isn't wise. If anyone can think of anyting I'm missing, I'd love to hear it.

Replies are listed 'Best First'.
Re: Clean out da hash...
by leriksen (Curate) on Feb 12, 2004 at 08:16 UTC
    For the bit where you do

    Delete everything in the hash

    wouldn't just 

    %hash = ();

    [download]

    work just as well ?

    +++++++++++++++++
    #!/usr/bin/perl
    use warnings;use strict;use brain;

[reply]
[d/l]
Re: Clean out da hash...
by ysth (Canon) on Feb 12, 2004 at 08:16 UTC
    I have a feeling that %hash is a my() variable and your clean_hash routine isn't in its scope. Are you using strict?

    The easier way to clear a hash is to say: %hash = ();

[reply]
[d/l]
      I thought 
      %hash = ();

      [download]
      would work, but it's not. And yes, I am using strict. The code is a bit long, but an abridged version is:
      use strict;
use warnings;

my %hash;

my @sets = qw(set_a set_b);

foreach my $set ( @sets ) {
    &find_start($set); #defines start and end points to grab data
    &fill_hash(); # fill 'er up
    &print_hash(); #print out
    &clean_hash(); #clean it out
}

      [download]
      The &find_start() looks throught the 6000 lines of text, and decides which line to start grabbing data, and which line to stopt at, based on the value of $set. I thought maybe I was grabbing the wrong stuff, but everything "adds up". Pun intended.
[reply]
[d/l]
[select]
        Are you absolutely certain those subs are using the same %hash as is declared there? (Try sticking some print \%hash; statements in the foreach loop and at the beginning of the *_hash subs.) Are the subs in the same source? Do you have another my %hash statement? You aren't tweaking the readonly flag on %hash, are you?
[reply]
[d/l]
        That doesn't show the code of &clean_hash, and that's where this is about.

        Could you provide us with a short, self contained program that shows that %hash = (); doesn't clear the hash in all cases?

        Abigail

[reply]

        Your code snippet is too short. I appreciate your desire to be brief, but we're missing some criticial bits, chief of which is the contents of the clean_hash subroutine.

        If I had to rely on intuition, though, I'd bet you were redeclaring your hash in clean_hash. This isn't a good or bad thing, entirely, because you should delete this call altogether, and replace it with the %hash=() that people have advised it'll get rid of your scoping error altogether. Wrapping such a trivial line of code in a sub makes baby Jesus cry.

[reply]
[d/l]
Re: Clean out da hash...
by calin (Deacon) on Feb 12, 2004 at 12:08 UTC
    Be aware that each keeps state, so adding or deleting elements in an each loop is not the greatest of ideas. However, your pattern of usage seems to be supported, though I wouldn't recommend it.

    Quote from perldoc -f each

    If you add or delete elements of a hash while you're iterating over it, you may get entries skipped or duplicated, so don't. Exception: It is always safe to delete the item most recently returned by "each()", which means that the following code will work: 

    while (($key, $value) = each %hash) {
  print $key, "\n";
  delete $hash{$key};   # This is safe
}

    [download]

[reply]
[d/l]
[select]
      "...is not the greatest of ideas."

      Why is it not the greatest of ideas when the docs specifically make it a point to say that it is "always safe to delete the item most recently returned by each()"? The OP's delete example is almost word for word the same as the one provided in the docs that you quoted.

      His problem lies elsewhere.

      He should be looking at scoping issues. He should also ween himself from passing variables into subs implicitly (as if they were globals) rather than explicitly as parameters to the subs. In this case, it would probably be wise to pass the hash into his delete sub by reference rather than global osmosis. But the act of deleting a hash's elements is so trivial that it hardly warrants its own subroutine.


      Dave

[reply]
        Big thanks to everyone. I finally got the problem figured out. A colleague across the ocean had some time on his hands and looked at it. It was a careless mistake on my side

        Early on in the script (before I changed strategies), I had two counters, to remind me what line I was on. I was assigning the wrong counter as the line number (I was off by one line) and it was pure coincidence that the text file was laid out in a manner where I was snagging a value that represented the sum of both sets of data. Chalk that one up to bad luck and carelessnes.

        I did learn a bit more about hashes though from the thread, so it was definitely not a lost cause. Also learned that if you change the idea of your script, it's not a bad idea to wipe the slate clean.
[reply]

Back to Seekers of Perl Wisdom