Gossamer Forum
Home : Products : Gossamer Links : Development, Plugins and Globals :

Re: [Paul] bug in dir Converter

Quote Reply
Re: [Paul] bug in dir Converter In reply to
Paul,
Here are my answers to your comments in post #3:

Quote:
1) "use lib" is used in the perl module but it isn't needed.
It is needed in my development process. Otherwise when checking for syntax with -c option I will get error. It can be commented out after release, though.
But causes NO problems.

Quote:
2) Data::Dumper debugging code is left in the code. That's not a major issue but it is loading a module that isn't needed. The reason for pointing this out too is that GT::Dumper is a better option for compatiblity sake (yes I know DAta::Dumper is standard but still).
Data::Dumper is used for debugging in development phase. I don't care about GT::Dumper (which module is not standard in Perl, but Data::Dumpler is). It can be commented out after release, tough. But since Data::Dumper is standard module in Perl releases will not cause any problems.
In overall, it causes NO problems.

Quote:
3) Excessive use of "my" at the top. my() is actually a sub-routine so there about 13 sub-routine calls when this could be reduced to one or two by declaring more than one variable at once.
You are right, that they could be declared in one step.
But usage of 13 "my" declarations does simply not matter in fact of the speed. The optimization you suggest is unnecessary, since in the full execution time these 13 "my"-s are executed in less than 1%. Optimization is unnecessary for such small speed gain.
Did you done profiling for DirName converter? I didn't, but I could say I'm 99% sure, that these 13 "my"s execution time, will be even not noted in the profiler result...
In overall the "my"s does NOT cause problems.

Quote:
4) "not" used where "!" would be more appropriate.
This does not mean any problems...
It's my programming style, and I like using not instead of ! for easier readibility.
Anyway it causes NO problems.

Quote:
5) Unnecessary escaping of single quotes inside qq||
Yeah, that's true. They are unnecessary. Earlier it was printed within '' quotes, then qq||; was added around the text later. Also I copied text from install.pm, where as you know '' quotes are used originally. I could correct all of them, but it does not cause any problems, so I don't bother about it currently.
In overall, it causes NO problems.

Quote:
6) Incorrect use of "ne" when comparing numerics. Should use "!="
Not incorrect. Both gives the same result, when comparing 2 integers.
When using 'eq' or 'ne' for number comparison, numbers are converted to strings automatically, then comparison is done as string.

Tough ne is likely slower a bit, but gives more readibility of the code (at least for me).
Finally since the speed decrease in under millisec, you don't have to bother about it. Also this plugin is not speed dependent, so speed does not matter.
In overall, it causes NO problems.

Quote:
7) Incorrect use of eq when comparing numerics. Should use "=="
Same as above, with the addition, that there are NO number comparisions using 'eq'!!!
You meant this code part?: $IN->param("step1") eq "1" .... also in html sourse: <INPUT TYPE="CHECKBOX" NAME="step1" VALUE="1">
All input parameters you get from the form are strings, so you statement is simply not true.
So it causes NO problems.

Quote:
8) Unnecessary use of parenthesis in regexes when no capturing is needed (this affects speed).
I checked the code, and yeah you are right, those parenthesis are unnecessary.
Those parentheses was used for grouping purpose, not for capturing reasons. I should use (?:) style groupings to avoid capturing or remove the parentheses from those regexps. Both would be saticfactory.
However yeah, the parenthesis capture affects the speed. But it does not mean so much speed decrease to bother with. Also DirName converter plugin has no speed demand at the moment.
I will place comments to the code to correct the captures, if there will be need for speed related optimizations in the plugin.

Quote:
9) Escpaing of forward slashes in regexes could be avoided by changing the substitution delimiter.
Yes, substitution delimiter could be changed. The code could be more readable.
However it causes NO problems in the code execution.

Quote:
10) Use of my() outside a sub-routine (thanks Yogi).
perldoc perlfunc says: "A my declares the listed variables to be local (lexically) to the enclosing block, file, or eval."
perdoc perlsub says: "You may declare my variables at the outermost scope of a file to hide any such identifiers from the world outside that file."
Having a my variable used in a module file is correct.
Paul, Yogi: you should try out this small example Tongue:

Code:
my $text = "Test my variable in file\n";
package Foo::Bar;
use strict;
print "Content-Type: text/plain\n\n";
print $text;
my $text = "Test my variable in function\n";
print $text;
So your comment was NOT valid in CGI mode. Unfortunately under mod_perl the my() declaration usage on file level will not work if it's used under mod_perl. The mod_perl is the only reason to correct it, and avoid usage of my() declarations on file level.

Quote:
I think the plugin would fail under mod_perl.
I went & read some docs, and it seems you are right.
But mod_perl is only reason not to use my() declarations on file level.


Final conclusions:
Paul,
Thanks very much for taking your precious time to find bugs in my plugin!
In overall, I aggreed you that may be problems in 1.5 points. In point 8) and (in case the plugin is executed under mod_perl) in point 10).
The point 8) affects the speed a bit, but does not affect the correct work of the plugin.
I would say, the only one point, which has potential problems is the point 10), which affects the mod_perl usage.
Anyway, I appreciate your constructive criticizm!

Go ahead Paul, do hundreds of tests on my plugin, discover all the possible problems it has, and tell me all the problems!
I always appreciate if somebody does my work for free. Cool

Best regards,
Webmaster33


Paid Support
from Webmaster33. Expert in Perl programming & Gossamer Threads applications. (click here for prices)
Webmaster33's products (upd.2004.09.26) | Private message | Contact me | Was my post helpful? Donate my help...

Last edited by:

webmaster33: Mar 13, 2003, 5:10 AM
Subject Author Views Date
Thread; hot thread bug in dir Converter ridesworld 9968 Mar 12, 2003, 8:07 AM
Post; hot thread Re: [ridesworld] bug in dir Converter
Andy 9660 Mar 12, 2003, 8:17 AM
Thread; hot thread Re: [ridesworld] bug in dir Converter
Paul 9683 Mar 12, 2003, 8:47 AM
Thread; hot thread Re: [Paul] bug in dir Converter
webmaster33 9553 Mar 13, 2003, 4:55 AM
Thread; hot thread Re: [webmaster33] bug in dir Converter
Paul 7273 Mar 13, 2003, 5:15 AM
Thread; hot thread Re: [Paul] bug in dir Converter
webmaster33 7277 Mar 13, 2003, 7:07 AM
Thread; hot thread Re: [webmaster33] bug in dir Converter
Paul 7280 Mar 13, 2003, 7:14 AM
Thread; hot thread Re: [Paul] bug in dir Converter
webmaster33 7192 Mar 13, 2003, 8:13 AM
Post; hot thread Re: [webmaster33] bug in dir Converter
klauslovgreen 7209 Mar 13, 2003, 8:40 AM
Thread; hot thread Re: [webmaster33] bug in dir Converter
Paul 7252 Mar 13, 2003, 9:03 AM
Thread; hot thread Re: [Paul] bug in dir Converter
webmaster33 7643 Mar 13, 2003, 9:29 AM
Post; hot thread Re: [webmaster33] bug in dir Converter
Paul 7202 Mar 13, 2003, 9:32 AM
Thread; hot thread Re: [ridesworld] bug in dir Converter
webmaster33 9689 Mar 12, 2003, 10:12 AM
Thread; hot thread Re: [webmaster33] bug in dir Converter
ridesworld 9672 Mar 12, 2003, 12:29 PM
Thread; hot thread Re: [ridesworld] bug in dir Converter
Paul 9777 Mar 12, 2003, 12:48 PM
Post; hot thread Re: [Paul] bug in dir Converter
webmaster33 9601 Mar 12, 2003, 1:08 PM
Thread; hot thread Re: [ridesworld] bug in dir Converter
webmaster33 9674 Mar 12, 2003, 1:11 PM
Thread; hot thread Re: [webmaster33] bug in dir Converter
yogi 9694 Mar 12, 2003, 1:19 PM
Thread; hot thread Re: [yogi] bug in dir Converter
Paul 9637 Mar 12, 2003, 1:26 PM
Thread; hot thread Re: [Paul] bug in dir Converter
webmaster33 9626 Mar 12, 2003, 1:54 PM
Thread; hot thread Re: [webmaster33] bug in dir Converter
Paul 9607 Mar 12, 2003, 2:10 PM
Thread; hot thread Re: [Paul] bug in dir Converter
webmaster33 9669 Mar 12, 2003, 2:22 PM
Thread; hot thread Re: [webmaster33] bug in dir Converter
Paul 9623 Mar 12, 2003, 2:27 PM
Post; hot thread Re: [Paul] bug in dir Converter
webmaster33 9560 Mar 12, 2003, 2:37 PM
Thread; hot thread Re: [yogi] bug in dir Converter
webmaster33 9606 Mar 12, 2003, 2:47 PM
Thread; hot thread Re: [webmaster33] bug in dir Converter
yogi 9624 Mar 12, 2003, 2:51 PM
Thread; hot thread Re: [yogi] bug in dir Converter
webmaster33 9625 Mar 12, 2003, 2:57 PM
Thread; hot thread Re: [webmaster33] bug in dir Converter
Paul 9629 Mar 12, 2003, 3:19 PM
Thread; hot thread Re: [Paul] bug in dir Converter
webmaster33 9613 Mar 12, 2003, 4:43 PM
Thread; hot thread Re: [webmaster33] bug in dir Converter
ridesworld 9684 Mar 13, 2003, 2:33 AM
Thread; hot thread Re: [ridesworld] bug in dir Converter
Paul 9592 Mar 13, 2003, 3:21 AM
Post; hot thread Re: [Paul] bug in dir Converter
webmaster33 9541 Mar 13, 2003, 3:35 AM
Post; hot thread Re: [ridesworld] bug in dir Converter
webmaster33 9514 Mar 13, 2003, 3:36 AM
Thread; hot thread Re: [webmaster33] bug in dir Converter
Paul 7247 Mar 13, 2003, 5:31 AM
Thread; hot thread Re: [Paul] bug in dir Converter
webmaster33 7257 Mar 13, 2003, 7:02 AM
Post; hot thread Re: [webmaster33] bug in dir Converter
Paul 7224 Mar 13, 2003, 7:07 AM
Thread; hot thread Re: [ridesworld] bug in dir Converter
webmaster33 7223 Mar 29, 2003, 10:00 AM
Thread; hot thread Re: [webmaster33] bug in dir Converter
cwi 7147 Mar 29, 2003, 10:07 AM
Thread; hot thread Re: [cwi] bug in dir Converter
webmaster33 7169 Mar 29, 2003, 10:15 AM
Thread; hot thread Re: [webmaster33] bug in dir Converter
Paul 7178 Mar 29, 2003, 11:20 AM
Thread; hot thread Re: [Paul] bug in dir Converter
webmaster33 7164 Mar 29, 2003, 12:13 PM
Thread; hot thread Re: [webmaster33] bug in dir Converter
Paul 7193 Mar 29, 2003, 12:37 PM
Thread; hot thread Re: [Paul] bug in dir Converter
webmaster33 7193 Mar 29, 2003, 1:09 PM
Thread; hot thread Re: [webmaster33] bug in dir Converter
Paul 7177 Mar 29, 2003, 1:15 PM
Thread; hot thread Re: [Paul] bug in dir Converter
webmaster33 7151 Mar 29, 2003, 1:20 PM
Post; hot thread Re: [webmaster33] bug in dir Converter
Paul 7107 Mar 29, 2003, 1:21 PM