Hi. The following snippet seems to be doing what I want, but I'd like
to get suggestions for refinement or improving the code. Thank you.
#!/usr/bin/perl -w
You enabled Warnings. Very good. However, these days, it is
preferable to write
use warnings;
rather than the -w option on the shebang.
use strict;
Excellent!
my $phrase = "MARKEM 1ST HOLDINGS INC. & MONEY MARKET LLC";
My personal preference is to not use double quotes unless you actually
need interpolation.
I get nervous when using the same identifier for multiple pieces of
information. There's nothing *wrong* with this, of course, and Perl
will readily do exactly what you want. It just seems needlessly
confusing to the reader. Consider something like `my @terms =
get_terms();` instead.
foreach my $term (@terms)
{
$phrase =~ s/\W/ /ig; #remove nonword characters
Your comment doesn't match what the code actually does. You're not
removing non-word characters. You're replacing each non-word character
with a single space character.
There is no reason for the /i modifier in that regexp.
You're specifying the /g modifer, which means this one single statement
removes *all* the non-word characters. So why is this line in a loop?
This code does not in any way depend on $term, and it should be moved
outside the loop so that it's only executed once.
$phrase =~ s/\S*\d\S*/ /ig; #remove numeric strings
Pretty much the same critiques for this one. The comment doesn't match
what's happening, the /i modifier is pointless, and it should only be
executed once.
$phrase =~ s/\b($term)\b//ig; #remove business terms
Okay. *This* one should absolutely remain in the loop, and should use
the /i modifier, and it is in fact actually removing the term.
My only critique here is that there's no reason to use (), as you're
not using $1 anywhere.
}
#new phrase: "MARKEM MONEY MARKET"
print "$phrase\n";
Again, your comment doesn't match. That is not the new phrase. The
new phrase is: 'MARKEM MONEY MARKET '
sub terms
{
my @terms = qw(holdings inc llc);
}
If this subroutine has no point other than returning a list of values,
why are you bothering to create a new variable at all?
Putting all my critiques together, we're left with:
#!/usr/bin/perl
use strict;
use warnings;
my $phrase = 'MARKEM 1ST HOLDINGS INC. & MONEY MARKET LLC';
my @terms = get_terms();
$phrase =~ s/\W/ /ig; #replace non-word chars with a space
$phrase =~ s/\S*\d\S*/ /ig; #replace strings containing a digit
#with a space
for my $term (@terms) {
$phrase =~ s/\b$term\b//ig; #remove business terms
}
#new phrase: 'MARKEM MONEY MARKET '
print "$phrase\n";
sub get_terms {
qw/holdings inc llc/;
}
__END__