[CDBI] Re: ANNOUNCE: Ima::DBI 0.35 released

Edward J. Sabol sabol at alderaan.gsfc.nasa.gov
Mon Jun 11 05:47:24 BST 2007


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



More information about the ClassDBI mailing list