Module

Marty Pauley marty+belfast-pm at kasei.com
Fri Aug 22 08:01:43 CDT 2003


On Fri Aug 22 13:08:26 2003, Scott McWhirter wrote:
> 
> before i venture this out into the public, i thought i'd let you rip it
> to shreds before putting it up when pause gets back online.

OK.

Your README is bad.  Potential users can see the README _before_ they
download your module, and they will use its contents to help decide to
use your module at all.  If you can't think of anything exciting to say
in your README, just do perldoc -t Match.pm > README

=head1 COPYRIGHT

You should license your module under GPL as well as Artistic license.

It is now 2003.

=head1 NOTES

I'm not convinced that your module users need to know this, especially
since your 'optimization' isn't.

You really shouldn't optimise before you have enough statistics to
demonstrate a problem.  Also, duplication of code is a much bigger
problem than execution speed.  Duplicated code is bad.

=head1 Your code

I assume you use qr because you think it will make your code faster.
In this case it will not: just use a normal regexp in your loop.
You should use qr if you want to compile a regex then use it many times.
You are compiling different regexes each time.

=head2 sub new

Your comment says "Safety first", but it is clearly the second
thing that happens.  If you're going to check your args, do it before
using them in your hashref.

Alternatively, you might want to get rid of the lexical variables and
use a hash slice to throw the args into $self before checking.

=head2 any / all / none

Repeated code is bad.  Repeated code is bad.  Repeated code is bad.

Look at how you've implemented 'count': it calls 'which' to do the work
and then checks the output.  Why not do the same sort of thing in these
other methods?

=head2 which

A map would be better than a foreach + push combo; the main point of map
is to replace foreach + push combos.

__END__

I haven't looked at your tests.

-- 
Marty



More information about the Belfast-pm mailing list