Best Practice for replace sub

[Skeeve]

Hi

I often find myself writing code like this:

# Encoding stuff####################################
my %ENCODE=(
    '"' => '"',
    '&' => '&',
    '<' => '&lt;',
    '>' => '&gt;',
    map((chr($_), "&#$_;"), (0..31)),
);
my $ENCODE= join('', keys %ENCODE);
$ENCODE= qr/([${ENCODE}])/;
sub encode {
    my($text)= @_;
    return '' unless defined $text;
    $text=~ s/$ENCODE/$ENCODE{$1}/g;
    return $text;
}
####################################################

Note: Please don't tell me about moduls being able to do exactly this encoding/replacements as this is just an example for several tasks where I need to replace substrings with other strings.

My bad feeling about this is: I'm using kind of global variables which I initialize once because I simply don't want to assign the hash (%ENCODE) and the accompanying scalar ($ENCODE) each time the encode($string) is invoked. So they need to be on global level. But it does not "feel right".

What's your proposal against this? Or what's your view on this?

[tobyink]

Perl 5.10 and above have state variables, but they don't work for hashes and arrays - just scalars. Still, you can use hashrefs/arrayrefs.

sub encode {
    state $lookup = {
        '"' => '"',
        '&' => '&',
        '<' => '&lt;',
        '>' => '&gt;',
        map((chr($_), "&#$_;"), (0..31)),
    };
    state $re = do { my $tmp = join('', keys %$lookup); qr/([$tmp])/ }
+;
    
    my ($text) = @_;
    return '' unless defined $text;
    $text =~ s/$re/$lookup->{$1}/g;
    return $text;
}

If you need to support older versions of Perl, then curly braces can be used to limit the scope of lexical variables...

{
    my $lookup = {
        '"' => '"',
        '&' => '&',
        '<' => '&lt;',
        '>' => '&gt;',
        map((chr($_), "&#$_;"), (0..31)),
    };
    my $re = do { my $tmp = join('', keys %$lookup); qr/([$tmp])/ };
    
    sub encode {
        my ($text) = @_;
        return '' unless defined $text;
        $text =~ s/$re/$lookup->{$1}/g;
        return $text;
    }
}

Or even better (because it delays populating the variables until they're needed):

{
    my ($lookup, $re);

    sub encode {
        $lookup ||= {
            '"' => '"',
            '&' => '&',
            '<' => '&lt;',
            '>' => '&gt;',
            map((chr($_), "&#$_;"), (0..31)),
        };
        $re ||= do { my $tmp = join('', keys %$lookup); qr/([$tmp])/ }
+;
    
        my ($text) = @_;
        return '' unless defined $text;
        $text =~ s/$re/$lookup->{$1}/g;
        return $text;
    }
}

[tmharish]

Closure (PM discussion here ) or make it a package?

[Anonymous Monk]

I'd put the encoding-work in a subroutine and give no further thought to initializing and tearing-down the variable. Unless you are truly doing this millions of times such that you can clearly demonstrate that there's a problem to be solved here, you have no problem. The code should be clear and well tested and easily changed, with low chances of compromising some other part of the system by doing so. This to me is an argument against "globals."

[tobyink]

In HTML::HTML5::Entities I've got a hash with 2231 entries. It would be wasteful in the extreme to rebuild that hash every time the encode_entities function is called.

Closing over a lexical variable is a perfectly safe thing to do.

(As it happens, I use a package variable for HTML::Entities because that's what HTML::Entities does, and the aim is to provide a compatible API.)