[CDBI] Re: ANNOUNCE: Ima::DBI 0.35 released
"Edward J. Sabol" <sabol-2oVx0MVsMgiP/[email protected]> Mon, 11 Jun 2007 00:47:24 -0400
| Newsgroups | gmane.comp.lang.perl.modules.class-dbi |
|---|---|
| Message-ID | <[email protected]> |
Matt Trout wrote:
>> But a global flag to say "look, just don't use prepare_cached" would also
>> be nice - the problem is that the choice is set at ->set_sql time so if your
>> superclass (whether CDBI or otherwise) says "yes" you have absolutely no
>> way to change your mind in a subclass, which is a complete pain.
Perrin Harkins replied:
> Class::DBI already overrides set_sql, so that seems like the
> appropriate place to put a configuration option like this.
I respectfully disagree. I think that it is more appropriate to include a way
of specifying the default for set_sql()'s $cache argument in Ima::DBI. It's
Ima::DBI's default value that people want to change, not Class::DBI's, and
Class::DBI's override method basically just passes all the arguments along to
Ima::DBI's anyway. And there's always some kind of wacko ;) who wants to use
Ima::DBI without Class::DBI, so why should Ima::DBI's set_sql() cache
statements in manner different from Class::DBI's set_sql()?
> Ima::DBI tries to be agnostic about using prepare_cached or not and just
> does what it's told.
Except that it defaults to 1 and there's no way to change that default; the
only alternative is to pass a value for $cache along with every call to
set_sql(). I think it makes sense for there to be a mechanism to change that
default (preferably on an inheritable per-class basis).
The changes to Ima::DBI amount to only a couple lines, I think. (Other than
docs and tests, of course.)
sub set_sql {
my ($class, $sql_name, $statement, $db_name, $cache) = @_;
$cache = 1 unless defined $cache;
...
Basically, that line of Ima::DBI's set_sql() method would become something
like the following:
$cache = (defined($class->_default_cache_setting)
? $class->_default_cache_setting : 1) unless defined $cache;
And earlier in the module there would be
Ima::DBI->mk_classdata('_default_cache_setting');
(I'm not advocating that it should necesarily be named
'_default_cache_setting'; that's just what I came up with off the top of
my head.)
What do you think? If someone were to submit a bug for Ima::DBI with a patch
like that (along with docs and tests)...
Just my two cents,
Ed