in reply to Hashes: Deleting elements while iterating

You probably want keys in here:

print "\nDelete children of $test :\n";
foreach $key (%index) {

[download]

That said, the sort loop won't do anything especially interesting to the loop body. The while construct is a little dodgy, as modifying a hash inside an iterator loop can often reset the iterator, effectively making an infinite loop. (Yes, each is an iterator. It grabs one bit of data at a time. foreach keys grabs all of the keys at once.)

Replies are listed 'Best First'.
Re: Re: Hashes: Deleting elements while iterating
by jarich (Curate) on Sep 04, 2003 at 03:26 UTC
    I think what chromatic was trying to say is: 
    print "\nDelete children of $test :\n";
foreach $key (keys %index) {
    ...
    delete $index{$key};
}

    [download]

    I suspect what is happening with your code: 

    foreach $key (%index) {
    delete $index{$key};
}

    [download]
    is the following. Remember that when you're dealing with foreach loops the item that you've got ($key in this case) is the item in the hash. That is, $key becomes an alias to the memory address within the hash.

    So what is happening is that you're generating a list of all the keys and values in the hash, which is a list of aliases to those and then you're screwing it up. Remember that the list gets generated right at the very start of the for/foreach loop's execution.

    You see, you delete a key, which means that your value gets deleted as well. Then you go onto a value and attempt to reference/delete that. All of a sudden you're attempting to delete something that you have a kind of reference to (from the generated list) but that has already been deleted from the hash! Hence, no doubt, the Attempt free unreferenced scalar at line 27. warning.

    No wonder that makes Perl sad!

    So, in conclusion, use keys. And remember that you should never fool around with the list you're interating over. I mentioned this recently over here too, although in a different case. I do accept that the best way to do what you need to do, is to iterate over your hash, just make sure that you're iterating over the keys of your hash, rather than the whole thing.

    All the very best and I hope this helped

    jarich

    Update: Upon further thought and a little discussion I should probably mention that delete doesn't work on values and wouldn't normally cause this error. As far as I can tell the warning is caused because we're trying to use a scalar (the value of a deleted key) that no longer exists. This was the idea I was trying to convey above, but I suspect I wasn't very clear.

    Update II: chromatic has corrected me by pointing out that he was merely pointing out that keys was missing from the original poster's loop in this case. Rather than showing where it should be used, which the original poster obviously would have known by using keys elsewhere.

[reply]
[d/l]
[select]
      Remember that when you're dealing with foreach loops the item that you've got ($key in this case) is the item in the hash. That is, $key becomes an alias to the memory address within the hash.

      I'm not sure that's accurate. Look at the last half of Perl_do_kv in doop.c and hv_iterkeysv and hv_iterval in hv.c. (I'm looking at something around 5.8.0.) The key iterator returns a mortal copy of the key, which is pushed onto the return stack. Sure, it points to the same data, but when you're done with it, it'll be ready for garbage collection without causing the original to be collected.

      The value is returned as a normal SV. I don't see any reference count increments, but there's very little you could do to cause it to be deleted from the hash without the associated key.

      I could really have misunderstood what you meant, but it doesn't sound quite right.

[reply]
[d/l]
[select]

In Section Seekers of Perl Wisdom