On 5/24/07, Peter Michaux <[EMAIL PROTECTED]> wrote:
Hi,
I'm writing a new version of JavaScript::Minification on CPAN. This is
my first CPAN module and first Perl project! If someone is willing to
take a look to see if I've done something terribly wrong packaging the
code I would greatly appreciate any feedback.
Currently the new module is in a subversion repository. If you have
subversion you should be able to check it out and test it with
svn co http://dev.michaux.ca/svn/random/JavaScript-Minifier
cd JavaScript-Minifier
perl MakeFile.PL
make test
Some comments on packaging:
* it may be a good idea to add a LICENSE parameter to Makefile.PL
(supported by ExtUtils::MakeMaker >= 6.31)
* you might like to add POD and POD coverage tests (for CPANTS' sake)
Some comments on code:
* it does not look like Perl code, but C code translated literally to Perl.
If you review the code using a Perlish approach, you may be surprised
about how short the result may be. For example,
sub isInfix {
my $x = shift;
return ($x eq ',' || $x eq '=' || $x eq ';' ||
$x eq '?' || $x eq ':' || $x eq '&' ||
$x eq '%' || $x eq '*' || $x eq '|' ||
$x eq '<' || $x eq '>' || $x eq "\n");
}
may be replaced by code using regular expressions
sub isInfix {
my $x = shift;
$x =~ /^[,=;?:&%*|<>\n]$/
}
But this in turn may be further improved by not working in a
character-by-character base as the current code does. For instance,
you may drop the anchors in the regexp above and even the subroutine
itself and replace it with the regexp itself (avoiding sub calls):
my $is_infix_re = qr/[,=;?:&%*|<>\n]/;
# and where you would say isInfix($x), the call would be replaced by
# $x =~ / $is_infix_re /xms or something like that
Best,
Adriano.
Thank you,
Peter
--
http://peter.michaux.ca
http://forkjavascript.org
