s/$key/$val/ in while each %hash

R

Robert Wallace

my program should replace
[ ] with a wingdings box
[*] with a wingdings checked box
[!] with another wingdings box
--> with a wingdings arrow

here's the hash:
%wingdings=(
'[*]' => '<font face="wingdings">þ</font>',
'[ ]' => '<font face="wingdings">o</font>',
'[!]' => '<font face="wingdings">q</font>',
'-->' => '<font face="wingdings">ð</font>',
);

sample string:
$x="[*] blah blah"

if sub called with:
print replace($x);

results:
0 [0] blah blah
since we can't display graphics, imagine 0 as the wingdings character
why do I get one item replaced twice. and why is one between the [ and
the ]?

sub replace {
while (($k,$v) = each %wingdings){
$str=~s/$k/$v/;
#print "key=$k, value=$v<br>\n"; # use for debuging
}
#print "$str<br>\n---------------------------<br>\n"; #use for
debuging
}

----------------------
and if the sample string is:
$x="[!] blah blah"

if sub called with:
print replace($x);

results:
[0] 0 blah blah
since we can't display graphics, imagine 0 as the wingdings character
why do I get one item replaced twice. and why is one between the [ and
the ]?
only, this time the [ and the ] are on the first replacement.

now if we set the s/// to do global replacement s///g, whoa!
we get every space replaced!
 
T

Tad McClellan

Robert Wallace said:
%wingdings=(


my %wingdings = (


You should always enable strict when developing Perl code.

Whitespace is not a scarce resource, feel free to use as much of
it as you want to make your code easier to read and understand.


'[*]' => '<font face="wingdings">þ</font>',
'[ ]' => '<font face="wingdings">o</font>',
'[!]' => '<font face="wingdings">q</font>',
'-->' => '<font face="wingdings">ð</font>',
);

sample string:
$x="[*] blah blah"

if sub called with:
print replace($x);

results:
0 [0] blah blah


The code you've shown does NOT make those results.

We will need to see the Real Code if you want a Real Answer.

since we can't display graphics, imagine 0 as the wingdings character
why do I get one item replaced twice.


You don't get one item replaced twice.

You are printing the return value from replace(), but it does
not explicitly return any value, so

print replace($x);

makes the same output as:

print 0;

sub replace {
while (($k,$v) = each %wingdings){
$str=~s/$k/$v/;


You have never put anything into $str.

Is this your Real Code?

You should always enable warnings when developing Perl code!

Have you seen the Posting Guidelines that are posted here frequently?



You have regex metacharacters in your patterns.

You must escape them if you want to match them literally:

$str =~ s/\Q$k/$v/;
 
R

Robert Wallace

Tad said:
......

$str =~ s/\Q$k/$v/;
\Q, smooth as a baby's rear-end.

how do you guys figure these things out.
I read the llama book. I read (and forgot) the perldocs.
well, you caught me, I'll remove my programmer costume/disguise, I
almost fooled myself.
I don't understand half of what's on perldoc.com.

as for my full code, its too Microsoftized; it's too big and ugly.
thought I'd make it simpler for people to see.
 
G

gnari

Robert Wallace said:
as for my full code, its too Microsoftized; it's too big and ugly.
thought I'd make it simpler for people to see.

ah yes, that is fine, but make a simple *real* program that actually runs.
often, just the process of creating a simple code that specifically
demonstrates the problem makes it clearer to the programmer,
and you might have figured it out for yourself. like in this particular
case,

you might have started with:
(note: I am not actually going to test anything here, but you should
always try to run things before posting)

use warnings;
use strict;
my $k='X';
my $v='Y';
my $str='X bla bla';
$str =~ s/$k/$v/;
print "[$str]\n";

you'll find it works. fine. add more things.
maybe make a replace() sub.

if that works, change the $k value and suddenly it does not work.
haha, you have some clues, and probably realize what kind of problem
this is and maybe hava an inkling of what perldoc to look into.

if you do not figure it out at that stage, you have a nice simple working
program to post here, and somone will spot your problem without having
to deduce what you are doing.

you will also find that people tend to be more positive if you show that
you have put some real effort into your post.

gnari
 
T

Toby

I'm learning Perl, using clpm as a resource. This post got me thinking a
bit (no, I didn't get the \Q answer), is there _any_ advantage to the
'while...%hash' construct?

My solution (before reading any followups) is replace2():

#!/usr/bin/perl
use warnings;
use strict;
use Benchmark ':all';

my %wingdings = (
'[*]' => '<font face="wingdings">þ</font>',
'[ ]' => '<font face="wingdings">o</font>',
'[!]' => '<font face="wingdings">q</font>',
'-->' => '<font face="wingdings">ð</font>',
);

my $x="[*] blah blah";

cmpthese(100000, {
replace => "replace(\"$x\")",
replace2 => "replace2(\"$x\")"
});

sub replace {
my $str = shift;
while (my ($k,$v) = each %wingdings) {
$str =~ s/\Q$k/$v/;
}
}
sub replace2 {
my $str = shift;
$str =~ s/([-\[].[\>\]])/$wingdings{$1}/g;
}
__END__

$ perl hash.pl
Rate replace replace2
replace 5319/s -- -84%
replace2 32895/s 518% --

Also, is there a better (neater) way of calling a sub with an argument
in cmpthese()? All the doc examples use anonymous subs.

Cheers
Toby
 
G

gnari

Toby said:
I'm learning Perl, using clpm as a resource. This post got me thinking a
bit (no, I didn't get the \Q answer), is there _any_ advantage to the
'while...%hash' construct?

My solution (before reading any followups) is replace2(): [snip]
$str =~ s/([-\[].[\>\]])/$wingdings{$1}/g;

this will match lots of unwanted strings like '-Z]' and '[->'


gnari
 
T

Toby

gnari said:
My solution (before reading any followups) is replace2(): [snip]
$str =~ s/([-\[].[\>\]])/$wingdings{$1}/g;

this will match lots of unwanted strings like '-Z]' and '[->'

Fair point; does this get closer:

#!/usr/bin/perl
use warnings;
use strict;

my %wingdings = (
'[*]' => '<font face="wingdings">þ</font>',
'[ ]' => '<font face="wingdings">o</font>',
'[!]' => '<font face="wingdings">q</font>',
'-->' => '<font face="wingdings">ð</font>',
);

my $re = join('|', keys %wingdings);
$re =~ s/([\[\]\*\>\!])/\\$1/g;

my $x="[*] blah blah";
replace2($x);

sub replace2 {
my $str = shift;
$str =~ s/($re)/$wingdings{$1}/;
print $str;
}
__END__

Or is this a case where readability / maintainability outweigh
performance? And there's gotta be a nicer way than the join and s///
that I've done ...

Sorry to bang on about this, but I've been trapped writing portable
/bin/sh scripts for a few years, Perl is like a breath of fresh air!

Cheers
Toby
 
J

Jay Tilton

: #!/usr/bin/perl
: use warnings;
: use strict;
: my %wingdings = (
: '[*]' => '<font face="wingdings">þ</font>',
: '[ ]' => '<font face="wingdings">o</font>',
: '[!]' => '<font face="wingdings">q</font>',
: '-->' => '<font face="wingdings">ð</font>',
: );
:
: my $re = join('|', keys %wingdings);
: $re =~ s/([\[\]\*\>\!])/\\$1/g;

[truncated]

: Or is this a case where readability / maintainability outweigh
: performance? And there's gotta be a nicer way than the join and s///
: that I've done ...

Use quotemeta() to escape metacharacters while building the regex
instead of performing the secondary s/// operation.

my $re = join '|', map quotemeta($_), keys %wingdings;
 
G

gnari

Toby said:
Fair point; does this get closer:
...
my $re = join('|', keys %wingdings);
$re =~ s/([\[\]\*\>\!])/\\$1/g;
you don't need to escape > and ! at all
you don't need to escape * and [ in a character class
so this should be enough:
$re =~ s/([\][*])/\\$1/g;
...
$str =~ s/($re)/$wingdings{$1}/;

you might want to use /g if there may be more than one match in string
$str =~ s/($re)/$wingdings{$1}/g;

gnari
 
G

gnari

Jay Tilton said:
...
Use quotemeta() to escape metacharacters while building the regex
instead of performing the secondary s/// operation.

my $re = join '|', map quotemeta($_), keys %wingdings;

indeed. specially, if you are likely to add to %wingdings or change one of
its keys, you do not want to have to check if it needs quoting.
this is much more maintainable.

gnari
 
T

Toby

Jay said:
Toby wrote:
:
: my $re = join('|', keys %wingdings);
: $re =~ s/([\[\]\*\>\!])/\\$1/g;

my $re = join '|', map quotemeta($_), keys %wingdings;

Thanks. map() is on my [long] list of Perl stuff to do. Seeing it in
a context I understand helps a lot.

Cheers
Toby
 
E

Eric Amick

I'm learning Perl, using clpm as a resource. This post got me thinking a
bit (no, I didn't get the \Q answer), is there _any_ advantage to the
'while...%hash' construct?

My solution (before reading any followups) is replace2():

#!/usr/bin/perl
use warnings;
use strict;
use Benchmark ':all';

my %wingdings = (
'[*]' => '<font face="wingdings">þ</font>',
'[ ]' => '<font face="wingdings">o</font>',
'[!]' => '<font face="wingdings">q</font>',
'-->' => '<font face="wingdings">ð</font>',
);

my $x="[*] blah blah";

cmpthese(100000, {
replace => "replace(\"$x\")",
replace2 => "replace2(\"$x\")"
}); [snip]
Also, is there a better (neater) way of calling a sub with an argument
in cmpthese()? All the doc examples use anonymous subs.

You could use

replace => 'replace($x)'

The single quotes prevent interpolation. Another possibility is

replace => sub{replace($x)}
 
E

Eric Amick

Toby said:
Fair point; does this get closer:
...
my $re = join('|', keys %wingdings);
$re =~ s/([\[\]\*\>\!])/\\$1/g;
you don't need to escape > and ! at all
you don't need to escape * and [ in a character class
so this should be enough:
$re =~ s/([\][*])/\\$1/g;

You don't even need to escape ] when it's the first character in a
character class:

$re =~ s/([][*])/\\$1/g;
 
T

Toby

gnari said:
Toby said:
Fair point; does this get closer:
...
my $re = join('|', keys %wingdings);
$re =~ s/([\[\]\*\>\!])/\\$1/g;
you don't need to escape > and ! at all
you don't need to escape * and [ in a character class
so this should be enough:
$re =~ s/([\][*])/\\$1/g;

Thanks. I've now read perlre again, and found that if ] is at the start
of the list in a character class, it doesn't need escaping either:

$re =~ s/([][*])/\\$1/g;

but quotemeta() has a new fan over here ...

Cheers
Toby
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,482
Members
44,901
Latest member
Noble71S45

Latest Threads

Top