Re: RFC: DBIx::Counter - persistent counter class with dbi storage

On Thu, 14 Apr 2005 01:44:43 +0200, Rhesa Rozendaal wrote:

>Before I go all the way by applying for a PAUSE account and uploading
>it, I would like to put it to public scrutiny first. I'd like to know
>I'm doing things properly from the start.

No need to wait, apply for one already.

>Here's my list of questions:
>- what about the module name? is it good?

I like it.

>- code review would be nice

Yes it would be. But it'll have to wait, I'll just comment on the API
for now.

>- documentation: clear enough?

Pretty much.

>- tests: for a module this simple?

Well, yes. Let the user make sure it works with his choice of DB. Of
course, you now have the problem of letting the user choose his test
database, which makes this not so trivial. Interactive tests are a pain
for CPAN-testers.

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

Whatever is easier for the user of your module. I don't expect to make
much of a difference, does it?

>Here's the pod:

>SYNOPSIS
>         use DBIx::Counter;
>         $c = new DBIx::Counter('my counter',
>                                 dsn       => 'dbi:mysql:mydb',
>                                 login     => 'username',
>                                 password  => 'secret'
>                               );

Hmm... did you forget the table name?

>         print $c->value;
>         $c->inc;

Perhaps combine the two, as well. But the user might be requesting for
the value more than once, and I don't think it should be incremented.
And he could forget incrementing it.

Here's an idea: let this object auto-increment the counter in the
database, if the user didn't do it already, when the object is
DESTROYed. Overridable (in the constructor), of course.
 
Or better yet, increment the database counter on constructor time, but
hold the old value in your object.

>     Connection settings can be set in the constructor, or by using the
>     package variables $DSN, $LOGIN and $PASSWORD and $TABLENAME. The table
>     name is configurable, but the column names are currently hard-coded to
>     counter_id and value. The following SQL statement can be used to create
>     the table:
>
>         CREATE TABLE counters (
>             counter_id  varchar(64) primary key,
>             value       int not null default 0
>         );

Uh... couldn't your module do this itself? Maybe for you it's a bit of a
portability problem for different databases, but it seems a bit awkward
for the user to have to handle this by hand, even if only once. It's too
low level to my taste.

-- 
	Bart.