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

Bart,

Thanks for your input!
I didn't expect this list to get me a response that quickly, seeing how low the traffic had been over the past few months. Glad to see it's 
still alive, because I learned a lot from reading it.

Bart Lateur wrote:
>>Here's my list of questions:
>>- what about the module name? is it good?
> 
> I like it.

Good :)

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

I figured it was a bit much to ask.

>>- 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.

I've been looking around a bit on CPAN to see how others do this.
I looked at DBIx::Sequence and some of the backends of CGI::Session. The approach seems to be to accept options to perl Makefile.PL and do 
extended tests from there. No command line options => no interactive tests.

I could also handle creating the table like that, perhaps, and even put the requested table name in the module as a default by rewriting it 
on the fly.
I'm not sure I could handle so much complexity right from the start, though.

>>- 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?

That's what I was thinking. I don't think it matters whether I croak, or DBI.

>>        $c = new DBIx::Counter('my counter',
>>                                dsn       => 'dbi:mysql:mydb',
>>                                login     => 'username',
>>                                password  => 'secret'
>>                              );
> 
> Hmm... did you forget the table name?

It's optional.

>>        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.

I'd rather keep them separate. I'd like to keep the possibility open for one application to increment the counter, and for another to poll 
the current value (i.e. read only access).

> Or better yet, increment the database counter on constructor time, but
> hold the old value in your object.

I have thought of that, but it would restrict the ways in which you can use this module.
I don't want to stress the inc() method over the dec() method, as both are equally valid.

Caching the value in the object is also something I chose not to do: I'd rather have value() return what's stored at that moment at the 
expense of performance.
If you don't care about accuracy that much, you can always keep the value in a temporary variable.

>>    counter_id and value. The following SQL statement can be used to create
>>    the table:
>>
> 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.

That's a valid point.
This should probably be part of the installation process. I'll have a closer look at some other modules, to see how they handle this.

Thank you very much for your feedback. It's much appreciated!

Rhesa