Topicalising a variable using "for" is apparently bad. Why?

Question

So I answered a question on SO and got a lot of flack for it. I have been using Perl for many years and use this topicalising quite a lot.

So let's start with some code. I am doing search and replace in these examples. The idea is to search for one and three from two strings and replace them.

$values = 'one two three four five';
$value2 = 'one 2 three four 5';

$values =~ s/one//g;
$values =~ s/three//g;

$values2 =~ s/one//g;
$values2 =~ s/three//g;

This code is simple and everyone accepts it.

I can also build an array or hash with a list of values to search and replace which is also acceptable.

However, When I build a script to topicalise $values and $values2 and lessen the amount of typing to build a script it seems to be misunderstood?

Here is the code.

$values = 'one two three four five';
$value2 = 'one 2 three four 5';

for ( $values, $values2 ) {
    s/one//g;
    s/three//g;
}

The above code will topicalise the variables for the duration of the for block, but many programmers are against this. I want to understand why this is unacceptable?

Answer 1

There are several points to consider.

---

Your code performs multiple substitutions on a list of variables. You can do that without using $_:

for my $s ($values, $values2) {
    $s =~ s/one//g;
    $s =~ s/three//g;
}

Personally I think nothing is wrong with the above code.

The general problem with $_ is that it's not a local variable. E.g. if the body of your for loop calls a function (that calls a function ...) that modifies $_ without localizing it (e.g. by assigning to it or using a bare s/// or using while (<...>)), then it will overwrite the variables you're iterating over. With a my variable you're protected because other functions can't see your local variables.

That said, if the rest of your code doesn't have this bug (scribbling over $_ without localizing it), $_ will work fine here.

---

However, the code in your answer people originally complained about is different:

for ($brackets) {
    s/\\left/(/g;
    s/\\right/)/g;
}

Here you're not trying to perform the same substitutions on many variables; you're just saving yourself some typing by getting rid of the $brackets =~ part:

$brackets =~ s/\\left/(/g;
$brackets =~ s/\\right/)/g;

Using an explicit loop variable wouldn't be a solution because you'd still have to type out $foo =~ on every line.

This is more a matter of taste. You're only using for for its aliasing effect, not to loop over multiple values. Personally I'm still OK with this.

Answer 2

perldoc perlsyn has this

The foreach is the non-experimental way to set a topicalizer.

The OP's construct is a perfectly valid way of writing Perl code. The only provisons I have regarding their earlier answer are

• Unlike the example here, only two operations were being applied to a single variable. That is only marginally briefer than simply writing two substitutions and I wouldn't bother here, although I may consider

  s/one//g, s/three//g for $value;
  

• Other than the topicaliser, the answer is identical to another one already posted. I don't believe this makes it sufficiently different to warrant another post