Or a growing dislike for how they're being used.
First, this isn't a statement about the quality of Class::DBI or DBIx::Class. They do what they're intended to do; well at that. I use Class::DBI and I'm starting to use DBIx::Class. If it weren't for sql->oop mappers, I never would have gotten this far this fast with Handel.
With that said, I'm really starting to dislike them. I think there's a dark side of them that no one talks about most of the time.
Yesterday I started a new quick and dirty photo gallery project under Catalyst. Before I even started anything web related, I wanted to get the core modules out of the way. For the point of this conversation, two of the main modules are Gallery and Photo; each tied to the galleries and photos tables in the database. One of the side goals was to be able to use Class::DBI or DBIx::Class as my sql abstraction layer. Through some use of base clases and some symbol table tom foolery, this actually worked without too much hassle.
But, here's where the dark side somes in: these mappers tend to have their own new/create subs and use them in their internals. That means that if you want you're own new/create sub to do things in addition to creating a new record, there's a good chance it just won't work right. Here's an example:
package MyApp::DBI; use base 'Class::DBI'; __PACKAGE__->connection(...); 1; package MyApp::Gallery; use base 'MyApp::DBI'; __PACKAGE__->table('galleries'); __PACKAGE__->columns(...); sub create { my ($self, $ops) = @_; my $dir = $ops->{'directory'}; if (-e $dir) { die 'directory exists'; else { mkdir $directory; $self->find_or_create($data); }; return; }; 1;
MyApp::Gallery->create will fail hard every time because the CDBI internals call create, but not the one you wish it would. In the case of Handel, I used new to create a new object (what a name for a constructor). A this point, It's just luck I didn't want a custom create() sub in my public API. More on public APIs later.
A similiar problem exists when using DBIx::Class and new:
package MyApp::DBI; use base 'DBIx::Class'; __PACKAGE__->connection(...); 1; package MyApp::Gallery; use base 'MyApp::DBI'; __PACKAGE__->table('galleries'); __PACKAGE__->add_columns(...); sub new { my ($self, $ops) = @_; my $dir = $ops->{'directory'}; if (-e $dir) { die 'directory exists'; else { mkdir $directory; $self->find_or_create($data); }; return; }; 1;
Since I'm trying to use either CDBI or DBIC, my problem is doubly compounded in terms of new/create. I would also have to tweak my new() to call insert() under DBIC, which breask the whole point of encapsulating the DBI layer.
So, here's my beef. These SQL->OOP mappers need to make their create/new subs private (_new, _create) in some way shape or form; yet still be subclassable. Breaking new() for the consumer is a sin in my book since it's the most common creation idiom out there. Using a mapper should NOT decrease my options in terms of what public subs I want in my API. The only thing I really need an sql->oop mapper to do is to provide methods to get/set fields, otherwise stay out of my way. There, I feel better.
Even if I weren't trying to use either/or dbi mapper, using one or the other with a custom new/create still could cause problems. There is a solution, but it means an extra layer of abstraction; sort of like the old n-tier approach under windows. MyApp::Gallery, MyApp::DBI::Gallery subclassing MyApp::DBI.
Let's look back to the CDBI example:
package MyApp::DBI; use base 'Class::DBI'; __PACKAGE__->connection(...); 1; package MyApp::DBI::Gallery; use base 'MyApp::DBI'; __PACKAGE__->table('galleries'); __PACKAGE__->columns(...); 1; package MyApp::Gallery; BEGIN { use 'MyApp::DBI::Gallery'; }; sub create { my ($self, $ops) = @_; my $dir = $ops->{'directory'}; if (-e $dir) { die 'directory exists'; else { mkdir $directory; return MyApp::DBI::Gallery->find_or_create($data); }; return; }; 1;
Now I've seperate my Gallery object from it's DBI layer. No sub name collisions. No CDBI/DBIC internals calling my subs when they really want theirs. Yeah!
With some tweaking, you could rebless that new gallery dbi instance into the current package and map the column accessors into the current package. Either way, there's no longer a collision of ideas. Gallery->new and Gallery->create are always mine and mine alone. There are no internals anywhere accidentally calling my subs.
Of course, I could be wrong. :-)
Now for my second sql->oop mapper rant. They foster bad software design. Before you flame me, think about this:
package MyApp::DBI; use base 'Class::DBI'; __PACKAGE__->connection(...); 1; package MyApp::SomeTable; use base 'MyApp::DBI'; __PACKAGE__->table('sometable'); __PACKAGE->has_many(tablelegs => 'MyApp::TableLegs'); 1;
Now, how does the consumer of your module add legs to a table? That's right, the CDBI specific method ->add_to_tablelegs({}). That's silly. What if I change the DB layer? You're stuck with a cruddy API, even if you map add_to_tablelegs to SomeOtherDBILayer::add_child_object(). MyApp::SomeTable should just have an add() method to abstract that out. How many people do that that use CDBI? Who knows. The same goes for find_or_create, searc_linke, and all the others. They're convience methods for the module writer, but should NEVER be exposed to the module consumer. The MyApp::Gallery, MyApp::DBI::Gallery ==> MyApp::DBI example above keeps the author and the consumers honest to a purposeful API.
Updated additional thoughts
package MyApp::Gallery; use MyApp::DBI::Gallery; sub new { my ($class, $opts) = @_; my $gallery = MyApp::DBI::Gallery->create($data); bless {gallery => $gallery), $class; } sub AUTOLOAD { my $self = shift; my $name = our $AUTOLOAD; if (UNIVERSAL::can($self->{gallery}, $name) { $self->{gallery}->$name(shift); }; };
-=Chris
|
|---|