davorg-cpan / symbol-approx-sub Goto Github PK

The tests are very much of the time they were written (or, perhaps, of my level of knowledge at the time they were written). There's a lot of stuff like ok(&sub eq 'string') which sohld be changed to is(sub(), 'string').

Also we should get rid of the explicit plans and use done_testing().

I was young(er) and stupid. Lots of the subroutine calls in this module are of the form &sub where they should be sub(). This should be fixed.

One of the tests includes a canon => ... option when loading Symbol::Approx::Sub. Symbol::Approx::Sub doesn't have a "canon" option.

I suspect that "canon" should be renamed to "transform". But more investigation is required.

This module used to be stored in a subversion repository. There are still some svn artifacts in the code (e.g. $Id$).

Check all of the files and remove these.

Currently we have our own code for loading plugins. Let's look at Module::Load or something like that instead.

(Bonus points for considering something like Module::Pluggable as well.)

A few tests just check $@ for truth. They should probably be more explicit about what they are testing for - otherwise tests might pass because things fail for the wrong reason.

Would be nice to use Text::Exception in the tests that throw exceptions.

We croak() in a few places - passing a string to croak(). It might be nicer to croak with real exception objects (something based on Exception::Class perhaps).

A couple more tests will get us to 100% coverage.

• matcher => undef
• chooser => sub { ... } (or, alternatively, chooser => \&some_chooser_sub)

The code in the generated AUTOLOAD has an obvious bug. Line 385 is currently

@match_ind = @subs[1 .. $#subs];

I think that should be.

@match_ind = 1 .. $#subs;

But I'd like to see a failing test before fixing it.

Missing tests:

• Invalid matcher
• Undef matcher
• Coderef chooser
• Invalid chooser
• Default chooser

README.xml was always a stupid idea. What's better - Markdown or Pod?

Currently most of the tests are just ok($something) or is($something, $something_else). It would be nice to add descriptions to the tests so a maintenance programmer has more chance of understanding what is going on.

This might be tricky. I'm not sure that I understand what all of the tests are doing currently :/