Is this trivial function silly?

Question

I came across a function today that made me stop and think. I can't think of a good reason to do it:

sub replace_string {
        my $string  = shift;
        my $regex   = shift;
        my $replace = shift;

        $string =~ s/$regex/$replace/gi;

        return $string;
}

The only possible value I can see to this is that it gives you the ability to control the default options used with a substitution, but I don't consider that useful. My first reaction upon seeing this function get called is "what does this do?". Once I learn what it does, I am going to assume it does that from that point on. Which means if it changes, it will break any of my code that needs it to do that. This means the function will likely never change, or changing it will break lots of code.

Right now I want to track down the original programmer and beat some sense into him or her. Is this a valid desire, or am I missing some value this function brings to the table?

Answer 1

The problems with that function include:

• Opaque: replace_string doesn't tell you that you're doing a case-insensitive, global replace without escaping.
• Non-idiomatic: $string =~ s{$this}{$that}gi is something you can learn what it means once, and its not like its some weird corner feature. replace_string everyone has to learn the details of, and its going to be different for everyone who writes it.
• Inflexible: Want a non-global search-and-replace? Sorry. You can put in some modifiers by passing in a qr// but that's far more advanced knowledge than the s/// its hiding.
• Insecure: A user might think that the function takes a string, not a regex. If they put in unchecked user input they are opening up a potential security hole.
• Slower: Just to add the final insult.

The advantages are:

• Literate: The function name explains what it does without having to examine the details of the regular expression (but it gives an incomplete explanation).
• Defaults: The g and i defaults are always there (but that's non-obvious from the name).
• Simpler Syntax: Don't have to worry about the delimiters (not that s{}{} is difficult).
• Protection From Global Side Effects: Regex matches set a salad of global variables ($1, $+, etc...) but they're automatically locally scoped to the function. They won't interfere if you're making use of them for another regex.

A little overzealous with the encapsulation.

Answer 2

print replace_string("some/path", "/", ":");

Yes, you get some magic in not having to replace / with a different delimiter or escape / in the regex.