Closure producing 'Modification of read-only value' error

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

#!/usr/bin/perl

use strict;
use warnings;

#my $number = get_number('one');
#print "$number\n";

for (qw(one two three)){
  my $number = get_number($_);
  print "$number\n";
}

{
  my $hash_ref;
  sub get_number{
    my ($num) = @_;
    unless (defined $hash_ref){
      while (<DATA>){
        chomp;
        my ($str, $num) = split /\|/;
        $hash_ref->{$str} = $num;
      }
    }
    return $hash_ref->{$num};
  }
}

__DATA__
one|1
two|2
three|3
[download]

This snippet produces the error message:

Modification of a read-only value attempted at closure_test.pl line 19
+.
[download]

Uncomment the two lines at the top of script and the output is as expected:

1
1
2
3
[download]

Can anyone help explain this behaviour?

Update 2

Ack! The first update was rubbish! Apologies.

Update

Deleted.

Comment on Closure producing 'Modification of read-only value' error Select or Download Code

Replies are listed 'Best First'.
Re: Closure producing 'Modification of read-only value' error by eyepopslikeamosquito (Archbishop) on Jun 11, 2005 at 08:47 UTC
The `$_` of your `for` loop is aliased to the read-only strings `qw(one two three)`. The `while` loop inside the sub overwrites `$_`, hence the "Modification of a read-only value attempted" error. If you add the line: `local $_;` [download] inside your `sub get_number` it works. Generally, mangling `$_` inside a subroutine is poor form. Update: a cleaner way to ensure the sub plays well with others is to simply change the while loop to use a lexical rather than the implicit `$_`: `while (defined(my $d = <DATA>)){ chomp $d; my ($str, $num) = split /\\|/, $d; $hash_ref->{$str} = $num; }` [download]	[reply] [d/l] [select]
Re^2: Closure producing 'Modification of read-only value' error by nobull (Friar) on Jun 11, 2005 at 11:04 UTC
No, don't do that. Never `local $_` in a subroutine. If you want to use the magic assignment you should do `local *_`. Of course you have to unpack anything you want from `@_` first. If you get into the habit of doing `local $_` inside you subroutines it will seem to work fine - maybe for years. Then one day someone will call one of your subroutines inside a `for()` loop that is iterating over the values of a tied array or hash. Very odd things will start happening in their program.	[reply] [d/l] [select]
Re^3: Closure producing 'Modification of read-only value' error by tlm (Prior) on Jun 11, 2005 at 16:39 UTC
What about the implicit localizations of `$_` done by `map`, `grep`, `for`, etc.? Do they do The Right Thing®? I think it is surprising (hence bad) that the `while ( <FH> )` idiom doesn't do an implicit localization of `$_` like the looping constructs listed above. Is there a fundamental reason for this, or is it due to a severe tuit shortage? the lowliest monk	[reply]
Re^4: Closure producing 'Modification of read-only value' error by nobull (Friar) on Jun 11, 2005 at 19:09 UTC
Re^3: Closure producing 'Modification of read-only value' error by kaif (Friar) on Jun 13, 2005 at 02:03 UTC
I'm sorry, maybe I'm dense, but what exactly is different when you iterate over a tied array or hash. What are these "very odd things" and why won't a simple `local $_` do? This is very interesting to me.	[reply] [d/l]
Re: Closure producing 'Modification of read-only value' error by tlm (Prior) on Jun 11, 2005 at 11:24 UTC
You can avoid modifying `$_` within the sub, by doing the initialization of `$hash_ref` in a `BEGIN` block: `{ my $hash_ref; BEGIN { while (<DATA>){ chomp; my ($str, $num) = split /\\|/; $hash_ref->{$str} = $num; } } sub get_number{ my ($num) = @_; return $hash_ref->{$num}; } }` [download] As a small bonus, the sub doesn't need to perform a test that would have been true only once. Update: In light of hv's comments below, I conclude that, in this case at least, saving the `while (<DATA>)` is not worth all the fuss and worry over `$_`. Hence in the revision below, the loop does not use `$_`. Read more... (466 Bytes) the lowliest monk	[reply] [d/l] [select]
Re^2: Closure producing 'Modification of read-only value' error by hv (Prior) on Jun 11, 2005 at 11:49 UTC
You can avoid modifying `$_` within the sub, by doing the initialization of $hash_ref in a BEGIN block Of course that doesn't avoid modifying `$_`, just moves the modification to BEGIN time - if this ends up in a module, for example, it will instead corrupt the caller's `$_` at the point they `require This::Module`, which is much nastier (and usually pretty hard for the caller to debug). I'd recommend always localising the scope of any changes to `$_`, with either a `local $_` or a `local *_` as appropriate. Hugo	[reply] [d/l] [select]