Rhesa Rozendaal writes:

> I wrote a little module ...  Before I go all the way by applying for a
> PAUSE account and uploading it ...

Hello there.  Neither of those are actually particularly arduous steps
to go through; merely saying that you have this module you wish to
contribute is sufficient to get you a Pause account.  And it's OK to
upload something you aren't yet convinced about: just label it as such
clearly at the top of its pod, and give it a 0.01 version number.

> I would like to put it to public scrutiny first.
> 
> - what about the module name? is it good?

It seems reasonable to me -- I pretty much guessed what the module does
just from its name.

> - code review would be nice

Sorry, I couldn't be bothered to download the tarball, open it up, and
look inside.  (If you'd already uploaded this to Cpan and linked to it
on the Cpan Search site then a handy link to view the source is provided
-- but that could just be me being particularly lazy, and it doesn't
necessarily follow that doing so will cause others to read your code.)

> - documentation: clear enough?

Made sense to me.

> - tests: for a module this simple?

Yes -- always have tests!  One approach would be to use DBD::SQLite for
the database, and skip the tests if that module isn't installed.

> - exception handling: should I do this myself, or let DBI throw them?

Put your own in if you can provide useful information.  But there isn't
much point in catching an another module's exceptions if all you're
going to do is throw them again unchanged.

>      Connection settings can be set in the constructor, or by using
>      the package variables $DSN, $LOGIN and $PASSWORD and $TABLENAME.

I don't see the advantage of the package variables, given you can set
all of these things in the constructor in a nice clean way.

>      value
>          returns the current value of the counter.
> 
>              print $c->value;
>              # or using overload:
>              print $c;

It would be good to emphasize that stringification is happening, and
that the value can be interpolated into strings:

  print "Item $c is being processed\n";

Cheers.

Smylers

Reply via email to