All the Perl that's Practical to Extract and Report
use Perl Log In
jk2addict (4946)
(email not shown publicly)
http://today.icantfocus.com/blog/
AOL IM: chrislaco (Add Buddy, Send Message)
Yahoo! ID: chrislaco@sbcglobal.net (Add User, Send Message)
Jabber: laco@scribblewerks.com
A Growing Dislike for SQL-OOP Mappers
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.
more 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);
};
};
an endless discussion thread about what features to include.
-- Jarkko Hietaniemi
Has-a, not is-a (Score:2)
Now that mapper can create whatever methods it wants, and you don't care. I don't think this has much to do with mappers in general as much as the ones you've chosen, which force you to subclass them.
Re:Has-a, not is-a (Score:1)
Tony
Re:Has-a, not is-a (Score:2)
But in that case most of your C::DBI classes would _just_ be the declarations, which isn't how I've ever seen anyone use it.
Use Method Dispatch! (Score:1)
I agree with you and Dave about the fragile inheritance problem (and wonder if adding hook methods [martinfowler.com] or migrating to a Class::Std [cpan.org]-based object model would solve some of these problems.
However, one of your suggestions makes a similarly unwarranted assumption, specifically:
What if I want to overload
can()? It's a method. Why shouldn't I be able to do that?Re:Use Method Dispatch! (Score:1)
Of course, I didn't have to check the method I'm referenceing exists at all. It just seemed like a safety measure. But then again, if you override anything in UNIVERSAL, its effects are your problem, not the modules authors problem right?
Or, was this comment geared towards the UNIVERSAL::can vs. $object->can debate that flares up from time to time?
Re:Use Method Dispatch! (Score:1)
Yes, it's the great "Why in the world are you explicitly calling the parent implementation of a method on an instance of a known-derived class?" debate. Someday I'm going to write
UNIVERSAL::newand write code that calls that directly instead of the actual constructors just to show how stupid a debate it is.Re:Use Method Dispatch! (Score:1)
I'll admit, I'm a tard when it comes to that debate. I've heard mutterings and seen people use can both ways. In an effort to make me less stupid, care to elaborate on the difference between the two in the context of that debate?
Re:Use Method Dispatch! (Score:1)
can()is an object method. There's a default implementation in theUNIVERSALbase class so that classes that don't need to define their own implementations can inherit it.Classes for which the default implementation is unsuitable can override it with their own implementations -- and there are good reasons for doing so, such as if you have a proxying or delegation relationship you want to keep transparent, if you autogenerate methods you can't know at compile time and don't necessarily want to install th
Re:Use Method Dispatch! (Score:1)
Would it be safe to assume that I should always use ->can against other classes, but if I was overriding and making my own can() sub, that the guts of my new and improved can() should use UNIVERSAL::can; or should it use $self->SUPER::can?
Re:Use Method Dispatch! (Score:1)
It's a normal method, so treat it as you would handle any inherited method. If you use
SUPER, the implementation in the parent can change but the child can continue to work without change.Re:Use Method Dispatch! (Score:1)
because the program will die unless
$scalaris an object or package name. If it's an unblessed reference or a simple scalar, calling a method throws an exception, so one ends up writing:which is just a lot of typing, and we kn
rjbs
Re:Use Method Dispatch! (Score:2)
Re:Use Method Dispatch! (Score:1)
By the way, note the use of UNIVERSAL::isa in perldoc -f ref.
rjbs
Re:Use Method Dispatch! (Score:1)
Don't worry; Rafael applied my patch to perlfunc a little while ago.
Re:Use Method Dispatch! (Score:2)
able is a good idea. Plus we need an option for Scalar::Util:
use Scalar::Util export => "able", to_namespace => "Customer::ProjectName"Re:Use Method Dispatch! (Score:1)
The other position as I see it can be summarized as, I think it more likely that some idiot who doesn't know about UNIVERSAL::can will write a can method that does the wrong thing, than that some smart guy will write a can method that works where UNIVERSAL::can would have failed.
Which is a vote of non-confidence in others, not a desire to see things break.
Re:Use Method Dispatch! (Score:1)
I can only see that argument as "Because someone else might write broken code, I'll deliberately write broken code."
Re:Use Method Dispatch! (Score:1)
See this thread [perlmonks.org] where multiple good programmers, having just been told what some of the problems are trying to mix AUTOLOAD and can, tried to write a can routine and got it wrong.
If good programmers who have just been told some of the big pitfalls can't do it, then why should I expect that anyone else will get it right?
The problem isn't with the maintainance programmer. It is with how a couple of independent features do not combine in any
Re:Use Method Dispatch! (Score:1)
If someone needs to override
can()and does it incorrectly, callingUNIVERSAL::can()will give the wrong answer. You can't fix that without fixing their code. That's their bug.If someone needs to override
can()and does it correctly, callingUNIVERSAL::can()will still give the wrong answer. How is that possibly good? What possible legitimate reason is there for recommending it? This technique has no chance of working correctly in the face of an overriddencan()! That's your bug.For the record, I'
Re:Use Method Dispatch! (Score:1)
By contrast you view it as something that is documented and supposed to work. If someone breaks it, you expect them to fix it. And since you expect people to fix it, you get upset if someone else bypasses that fix.
To me that seems like a lot of stress and eff
Sick (Score:1)
I could use Class::DBI, DBIx::Class, Rose::DB::Object, or my own raw SQL. I can change DBI layers at will and not have to fight my MyApp::Gallery class or whatever is-a relationship it would've had normally.
There's nothing new about this approach. True n-tier development does that forever. But in the lasd of "just use base 'Class::DBI' in your class and go", it's somewhat interesting.
Get out of the way (Score:1)
You really don't want to be using Class:DBI, then...
Tony