Beefy Boxes and Bandwidth Generously Provided by pair Networks
Just another Perl shrine
 
PerlMonks  

Re: Bench this BBcode Part 2

by jwkrahn (Monsignor)
on Oct 03, 2010 at 17:44 UTC ( #863192=note: print w/ replies, xml ) Need Help??


in reply to Bench this BBcode Part 2

$ grep -e /o -e /go -e /io -o AUBBC.pm | wc -l 89

You are using the /o option 89 times, mostly on regular expressions that do not contain variables, but the /o option is only useful on regular expressions that do contain variables.    Perhaps you should read: What is /o really for?


12 my %AUBBC = (aubbc => 1,utf => 1,smileys => 1,highlight => 1,h +ighlight_function => \&{code_highlight},no_bypass => 0,for_links => 0 +,aubbc_escape => 1,no

It is more readable to put one key/value pair per line, for example like:

my %AUBBC = ( aubbc => 1, utf => 1, smileys => 1, highlight => 1, highlight_function => \&{ code_highlight }, no_bypass => 0, for_links => 0, aubbc_escape => 1, no_img => 0, icon_image => 1, image_hight => 60, image_width => 90, image_border => 0, image_wrap => ' ', href_target => ' target="_blank"', images_url => '', html_type => ' /', fix_amp => 1, line_break => 1, code_class => '', code_extra => '', href_class => '', quote_class => '', quote_extra => '', script_escape => 1, protect_email => 0, email_message => 'Contact&#3 +2;Email', highlight_class1 => '', highlight_class2 => '', highlight_class3 => '', highlight_class4 => '', highlight_class5 => '', highlight_class6 => '', highlight_class7 => '', );

14 my $long_regex = '[\w\.\/\-\~\@\:\;\=]+(?:\?[\w\~\.\;\:\,\$\-\ ++\!\*\?\/\=\&\@\#\%]+?)?';

Perhaps you should compile this once with the qr operator instead of every time you use it?


15 my @key64 = ('A','B','C','D','E','F','G','H','I','J','K','L',' +M','N','O','P','Q','R','S','T','U','V','W','X','Y','Z','a','b','c','d +','e','f','g','h','i'

That is usually written as:

my @key64 = ( 'A' .. 'Z', 'a' .. 'z', 0 .. 9, '+', '/' );

42 sub settings { 43 my ($self,%s_hash) = @_; 44 if (keys %s_hash) { 45 foreach (keys %s_hash) { 46 if ('highlight_function' eq $_) { 47 my $is_ok = check_subroutine($s_hash{$_}) || ''; 48 $AUBBC{highlight} = 0; 49 $AUBBC{highlight_function} = ($is_ok) ? \&{$s_hash{$_}} : +\&{code_highlight}; 50 } else { 51 $AUBBC{$_} = $s_hash{$_}; 52 } 53 } 54 &settings_prep;

Your if block starting at line 44 is superfluous because a foreach list with 0 elements will execute 0 times so you are unnecessarily running keys %s_hash twice.    Line 51 reveals that major purpose of the loop is to copy the keys and values of %s_hash to %AUBBC so I would write that something like:

sub settings { ( my $self, %AUBBC ) = ( @_, %AUBBC ); if ( exists $AUBBC{ highlight_function } ) { $AUBBC{ highlight_function } = \&code_highlight unless check_s +ubroutine( $AUBBC{ highlight_function } ); $AUBBC{ highlight } = 0; } settings_prep();

Or if you need the new values in @_ to overwrite the old values in %AUBBC:

sub settings { my $self = shift; %AUBBC = ( %AUBBC, @_ ); if ( exists $AUBBC{ highlight_function } ) { $AUBBC{ highlight_function } = \&code_highlight unless check_s +ubroutine( $AUBBC{ highlight_function } ); $AUBBC{ highlight } = 0; } settings_prep();

169 $protect_email = '[' if $AUBBC{protect_email} eq 3 || $AUBBC{ +protect_email} eq 4; 171 if ($AUBBC{protect_email} eq 1 || $AUBBC{protect_email} eq 2 +) { 173 } elsif ($AUBBC{protect_email} eq 3) { 175 } elsif ($AUBBC{protect_email} eq 4) { 180 if ($AUBBC{protect_email} eq 1) { 182 } elsif ($AUBBC{protect_email} eq 2) { 185 } elsif ($AUBBC{protect_email} eq 3 || $AUBBC{protect_email} +eq 4) { 188 return <<JS if $AUBBC{protect_email} eq 2 || $AUBBC{protect_e +mail} eq 3 || $AUBBC{protect_email} eq 4; 224 /eg if ($Build_AUBBC{$_}[1] eq 1); 229 /eg if ($Build_AUBBC{$_}[1] eq 2); 234 /eg if ($Build_AUBBC{$_}[1] eq 3); 236 $msg =~ s/\[$_\]/$Build_AUBBC{$_}[2]/g if ($Build_AUBBC{$_}[ +1] eq 4); 256 if ($NewTag{type} ne 4) { 260 $NewTag{pattern} = 'l' if ($NewTag{type} eq 3 || $NewTag{type +} eq 4); 374 (!$option && $AUBBC{line_break} eq 2) 376 : $text =~ s/\n/<br$AUBBC{html_type}>\n/go if !$option && $ +AUBBC{line_break} eq 1;

You are using a string operator on a number which means that Perl has to convert the number to a string first before the comparison.    You should either use a numerical comparison operator on numbers (== instead of eq) or use strings when using string comparison operators ($var eq '3').


222 my $ret = do_sub( $_, $2 , $Build_AUBBC{$_}[2] ) || ''; 223 $ret ? $ret : $1; 227 my $ret = do_sub( $_, $2 , $Build_AUBBC{$_}[2] ) || ''; 228 $ret ? $ret : $1; 232 my $ret = do_sub( $_, '' , $Build_AUBBC{$_}[2] ) || ''; 233 $ret ? $ret : $1;

Or simply:

do_sub( ... ) || $1;

243 $fun = \&{$fun};

Why are you creating a new reference to an existing code reference?



Comment on Re: Bench this BBcode Part 2
Select or Download Code
Re^2: Bench this BBcode Part 2
by $h4X4_&#124;=73}{ (Novice) on Oct 04, 2010 at 09:16 UTC
    Thanks for the tips I will play around with a few ideas you have given me ^^

    You are using a string operator on a number which means that Perl has to convert the number to a string first before the comparison. You should either use a numerical comparison operator on numbers (== instead of eq) or use strings when using string comparison operators ($var eq '3').

    It has to be a string operator because like you said "Perl has to convert the number to a string first" and I need to use eq in case a developer flip flops from strings to numerical, so there is no errors from warn. So like you said "($var eq '3')" is what needs to be done there.

    Why are you creating a new reference to an existing code reference?

    At that point its still not a code ref. its the text name of it. I think you didn't understand what check_subroutine does but that gave me an idea for the code to do less.

    Stewie: You know, I rather like this God fellow. Very theatrical, you know. Pestilence here, a plague there. Omnipotence ... gotta get me some of that.
Re^2: Bench this BBcode Part 2
by JavaFan (Canon) on Oct 04, 2010 at 10:58 UTC
    Perhaps you should compile this once with the qr operator instead of every time you use it?
    No, he shouldn't. That *slows* down the program (although probably not measurable). I see $long_regex only be used twice, and in both cases it's interpolated. There's no point in making the compile-stringify round trip, if all you're needing is the string form. (On top of that, after the roundtrip, the resulting string is larger).

    Using qr to avoid additional compilation only makes sense if you don't interpolate, and use it in different places.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://863192]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others scrutinizing the Monastery: (9)
As of 2014-08-29 08:26 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    The best computer themed movie is:











    Results (277 votes), past polls