RFC: Simple DBI abstraction

Jaap has asked for the wisdom of the Perl Monks concerning the following question:

Wise Monks,

I made a module that is supposed to be an interface between a perl program and the DBI module. The basic idea is that there is just 1 method, named do() that you use to get or set data.

The do() method takes 2 or 3 arguments. The first argument is the table name, the second argument is a reference to a hash with key-value pairs to search for. (key is column name)
The third (optional) argument is a reference to a hash of key-value pairs to set/insert/update.

A main program using this module could look like this:
_{Edited to comply to Juerd and iburrell ;)}

use DBIx::Easy;

my $dbez = DBIx::Easy->new("DBI:mysql:database=mydb;host=localhost", '
+dbusername', 'dbpassword');
foreach ($dbez->do(tableName => 'mytable', get => {'column_name' => 'v
+alue', 'another_column_name' => 'another value'}, set => {'column_nam
+e' => 16}))
{
        foreach my $key (keys %$_)
        {
                print "$key: ", $$_{$key} || '', "\t";
        }
        print "\n";
}
[download]

The module code is below:

package DBIx::Easy;
use strict;
use warnings;
use DBI;

sub new
{
        my $class = shift;
        my @connectionParameters = @_;
        my $self = {};
        bless($self, $class);
        $self->{'dbh'} = DBI->connect(@connectionParameters);
        return $self;
}

sub do
{
        my $self = shift;
        my $tableName = shift;
        my $getRef = shift;
        my $setRef = shift;

        if ($setRef && ref($setRef) eq 'HASH' && %$setRef)
        {
                if ($getRef && ref($getRef) eq 'HASH' && %$getRef)
                {
                        ### update
                        my @results = $self->select($tableName, $getRe
+f);
                        $self->update($tableName, $getRef, $setRef);
                        for (my $i = 0; $i < @results; $i++)
                        {
                                foreach (keys %$setRef)
                                {
                                        $results[$i]->{$_} = $$setRef{
+$_};
                                }
                        }
                        return @results;
                }
                else
                {
                        ### insert
                        $self->insert($tableName, $setRef);
                        return ($setRef);
                }
        }
        ### select
        return $self->select($tableName, $getRef);
}
sub select
{
        my $self = shift;
        my $tableName = shift;
        my $dataRef = shift;
        my $dbquery = "select * from $tableName";
        my $counter = 0;
        my @placeholder = ();
        foreach (keys %$dataRef)
        {
                $dbquery .= $counter ? ' and ' : ' where ';
                $dbquery .= "$_=?";
                push (@placeholder, $$dataRef{$_});
                $counter++;
        }
        my $sth = $self->{'dbh'}->prepare($dbquery);
        $sth->execute(@placeholder);
        my @result;
        while (my $ref = $sth->fetchrow_hashref())
        {
                push (@result, $ref);
        }
        $sth->finish();
        return @result;
}
sub insert
{
        my $self = shift;
        my $tableName = shift;
        my $dataRef = shift;
        my $dbquery = "insert into $tableName(";
        $dbquery .= join(", ", (keys %$dataRef));
        $dbquery .= ") values(";
        $dbquery .= join (", ", map('?', (keys %$dataRef)));
        $dbquery .= ")";
        my $sth = $self->{'dbh'}->prepare($dbquery);
        $sth->execute(values %$dataRef);
}

sub update
{
        my $self = shift;
        my $tableName = shift;
        my $getRef = shift;
        my $setRef = shift;
        #$rows_affected = $dbh->do("UPDATE your_table SET foo = foo + 
+1");
        #UPDATE search SET Category = 'Java', Test = '1' WHERE Categor
+y = 'Jive' AND Test = '2';
        my $dbquery = "update $tableName set ";
        $dbquery .= join(", ", map ("$_=?", keys %$setRef));
        $dbquery .= " where ";
        $dbquery .= join(" and ", map ("$_=?", keys %$getRef));
        my $sth = $self->{'dbh'}->prepare($dbquery);
        $sth->execute(values %$setRef, values %$getRef);
}
sub DESTROY
{
        my $self = shift;
        $self->{'dbh'}->disconnect();
}
[download]

Any comments would be highly appreciated.

Comment on RFC: Simple DBI abstraction Select or Download Code

Replies are listed 'Best First'.
Re: RFC: Simple DBI abstraction by Juerd (Abbot) on Sep 08, 2004 at 22:10 UTC
Code contains many hints to what happens. Or at least, it should. I think removing the word that describes what happens is a bad idea. Insert/select/update all make it very clear what goes on. "do" does not. It is like having a $data. Nice, but what kind of data? With do, I'd want to know what it does. Choosing based on something as subtle as argument differences isn't good enough, in my opinion. There's a lot that can be abstracted in SQL. SQL::Abstract and DBIx::Abstract abstract the query into a method name and data structures, Class::DBI abstracts everything into objects. But always there is a way to see exactly what goes on. The action should never be left out, and always be explicitly stated. Please have a look at DBIx::Simple. It can use SQL::Abstract for some abstraction, but you'll always have to tell it what to do. It won't try to choose on its own. Which brings me to another point: IF you decide to put this on CPAN, please make sure it is in the DBIx:: namespace. Don't invent a new root namespace just because you couldn't think of a name in DBIx::, because DBIx::Abstract was already taken. Be creative. (How about DBIx::Do?) Juerd # { site => 'juerd.nl', plp_site => 'plp.juerd.nl', do_not_use => 'spamtrap' }	[reply]
Re^2: RFC: Simple DBI abstraction by Jaap (Curate) on Sep 09, 2004 at 08:13 UTC
You make a good point here, Juerd. How about i change the do() arguments to this: `$dbh->do(tableName => "table_name", get => {...}, set => {...})` [download]	[reply] [d/l]
Re^3: RFC: Simple DBI abstraction by Juerd (Abbot) on Sep 09, 2004 at 13:35 UTC
You make a good point here, Juerd. How about i change the do() arguments to this: $dbh->do(tableName => "table_name", get => {...}, set => {...}) Better already, but when in real life is it practical to simultaneously set and get? I can't think of any situation. Thus abstracting it more, to two methods instead of one, makes sense to me: `$dbh->get("table_name", {...}); $dbh->set("table_name", {...});` [download] Which is very close to what DBIx::Simple lets you write if you have SQL::Abstract installed: `$db->select('table_name', [...]); $db->update('table_name', {...}, ...);` [download] Juerd # { site => 'juerd.nl', plp_site => 'plp.juerd.nl', do_not_use => 'spamtrap' }	[reply] [d/l] [select]
Re^4: RFC: Simple DBI abstraction by Jaap (Curate) on Sep 09, 2004 at 16:20 UTC
•Re: RFC: Simple DBI abstraction by merlyn (Sage) on Sep 08, 2004 at 20:44 UTC
I think you'd be very happy (and in good company) with Class::DBI. You can use just the parts of it that do what you want, but the rest of it is there to make things even better when you're ready. -- Randal L. Schwartz, Perl hacker Be sure to read my standard disclaimer if this is a reply.	[reply]
Re^2: RFC: Simple DBI abstraction by Jaap (Curate) on Sep 08, 2004 at 20:46 UTC
I know Class::DBI and i use it frequently. It is a very good piece of software. Still if you have any comments on my code, it'd be appreciated.	[reply]
•Re^3: RFC: Simple DBI abstraction by merlyn (Sage) on Sep 08, 2004 at 20:50 UTC
My comment on your code is to not use your code and go look at Class::DBI instead. You are reinventing needlessly, unless there's some aspect of your code that solves a particular problem even better than CDBI does it, sacrificing all the common knowledge gained from CDBI's existence and community. -- Randal L. Schwartz, Perl hacker Be sure to read my standard disclaimer if this is a reply.	[reply]
Maybe too simple by htoug (Deacon) on Sep 09, 2004 at 06:05 UTC
I feel that it is just too simple. It might work in an environment where you never have to do a join or in other ways really use the fact that you have a database system out there with a much greater potential. But maybe I am predjudiced, I work in a place where our database (yes we only have one for all our data, although we do have copies for analysis, reporting etc, and a smaller version for development and testing) has over 1500 tables and is currently at ~50GB and growing. I my experience, as soon as you grow beyond using the database as a way of just storing simple data, you will have joins in most of your queries, and so very little use for Class::DBI and related socalled simple tools. The logical and the physical models are just too different! Your application needs data that is in a form that is suitable for its needs, the database needs the data to be normalised and easy to access for many different purposes. Your 'return on investment' will be much greater then. /me leaving soapbox and returning to lurkmode.	[reply]
Re: Maybe too simple by Juerd (Abbot) on Sep 09, 2004 at 13:40 UTC
I my experience, as soon as you grow beyond using the database as a way of just storing simple data, you will have joins in most of your queries, and so very little use for Class::DBI and related socalled simple tools. Class::DBI is great if you see it as an SQL based object persistence framework. If you try to use it as a replacement for SQL itself, it suddenly becomes a whole lot less useful. It's not a querying language - in fact, that is what it tries very hard to not be :) About so-called "simple" tools, I'm sure you will find that a simple interface can very well be used in complex designs, having looked at DBIx::Simple. It doesn't abstract, it simplifies. Please don't see one as the synonym of the other. Juerd # { site => 'juerd.nl', plp_site => 'plp.juerd.nl', do_not_use => 'spamtrap' }	[reply]
Re: RFC: Simple DBI abstraction by CountZero (Bishop) on Sep 08, 2004 at 21:18 UTC
The idea of having only one method, which itself decides whether you want to do a SELECT, INSERT or UPDATE is rather well-found. On the other hand, the fact that you can only search on "equal" and not on "like" or other operators, seems artificially limiting. Id. goes for only using "AND" between search terms. Perhaps if one provided a scalar instead of a hash ref as second argument, this would signal that in this scalar is to be found a verbatim "WHERE"-clause to be used as such. Mind you , it would make your module less secure as it would be possible to inject arbitrary SQL-code into the statement! CountZero "If you have four groups working on a compiler, you'll get a 4-pass compiler." - Conway's Law	[reply]
Re^2: RFC: Simple DBI abstraction by Jaap (Curate) on Sep 08, 2004 at 21:52 UTC
Thanks for your comment, CountZero. I'm bothered by the lack of `like` searches just like you. Still thinking about that. As for the AND/OR: i find myself using AND 90% of the time, but i'm gonna think about it. You could always do 2 separate searches though.	[reply] [d/l]
Re: RFC: Simple DBI abstraction by iburrell (Chaplain) on Sep 09, 2004 at 16:44 UTC
Don't use `$dbh` to name the objects, or `do` as the method name. $dbh is the standard idiom for DBI database handles. If you call your objects $dbh even in sample code, then people will get confused and think they are real database handles. And `do` is a method on database handles. `$dbh->do` implies the standard do method. Use another, more descriptive name. If you can find a better name, maybe that is a good sign the interface needs some work.	[reply] [d/l] [select]