Beefy Boxes and Bandwidth Generously Provided by pair Networks
There's more than one way to do things
 
PerlMonks  

A Growing Dislike for SQL-OOP Mappers

by jk2addict (Chaplain)
on Aug 22, 2005 at 14:02 UTC ( [id://485692]=perlmeditation: print w/replies, xml ) Need Help??

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

Replies are listed 'Best First'.
Re: A Growing Dislike for SQL-OOP Mappers
by zby (Vicar) on Aug 22, 2005 at 15:00 UTC
    Just one word: triggers.

    From the Class::DBI documentation:

    TRIGGERS __PACKAGE__->add_trigger(trigger_point_name => \&code_to_execute); # e.g. __PACKAGE__->add_trigger(after_create => \&call_after_create) ; It is possible to set up triggers that will be called at various point s
    Triggers are the database way of running some extension code upon creating a record and Class::DBI tries to do it in a manner as similar as possible.

      First make one thing clear for people who don’t know: those triggers are at application level, not the database level. In general, I am against the idea of using those triggers, unless the database itself does not support database level trigger. Those triggers actually could be much more complex than it first looks like, especially when data integrity becomes a concern and complex locking issues. When you deal with database, leave as much as possible to the database.

        i agree (leave as much as possible to the db -- of roucse there are exceptions to every rlue as well), BUT in reality sometimes that's just not possible. Reasons could include db that doesn't support it, a DBA that doesn't support it, a DBA that can't support it, lack of a DBA altogether, or some combination of those or other reasons. (yes, crappy, but those conditions are out there)

      Sure....if I'm using CDBI.

      That doesn't change the overall premise. Your modules public API should not be the DBI layers API.

Re: A Growing Dislike for SQL-OOP Mappers
by siracusa (Friar) on Aug 22, 2005 at 17:27 UTC
    Maybe you're just using the wrong RDBMS-OO mappers? ;)
    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.

    I agree about keeping new() as simple as possible. It's a lot easier to decrease API granularity in your subclass than it is to increase it. Here's an example using a different RDBMS-OO mapper that has a finer API granularity than Class::DBI when it comes to object construction.

    Set up abstracted data source:

    package MyApp::DB; use Rose::DB; our @ISA = qw(Rose::DB); MyApp::DB->register_db( domain => 'default', type => 'default', driver => 'mysql', database => 'mydb', host => 'localhost', username => 'someuser', password => 'mysecret'); 1;

    Set up base class for all DB-backed objects:

    package MyApp::DB::Object; use Rose::DB::Object; our @ISA = qw(Rose::DB::Object); sub init_db { MyApp::DB->new } 1;

    Finally, the example gallery object class with the customized constructor:

    package MyApp::DB::Gallery; use MyApp::DB::Object; our @ISA = qw(MyApp::DB::Object); __PACKAGE__->meta->table('galleries'); __PACKAGE__->meta->columns(...); sub new { my($class, %args) = @_; my $dir = $args{'directory'}; if (-e $dir) { die "directory '$dir' exists"; } mkdir $dir or die "mkdir($dir) - $!"; # The following is roughly equivalent to find_or_create() my $self = $class->SUPER::new(%args); $self->load(speculative => 1); # Find... if($self->not_found) { $self->save; # ...or create } return $self; } 1;

    A little different than what you're used to, perhaps, but there it is.

    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, search_like, and all the others. They're convenience methods for the module writer, but should NEVER be exposed to the module consumer.

    I'm not a fan of "add related object" APIs that are auto-created by default. Having method generators available for those abilities is fine, but it's rare that an RDBMS-OO mapper has enough information to correctly guess when and how to do such things--let alone what to name the methods. Your example illustrates this.

    What I would probably do is write the method myself, since it's so simple:

    package MyApp::Table; ... sub add_leg { my($self, $leg) = @_; $leg->table_id($self->id); # tie leg to this table $leg->save; push(@{$self->{'legs'}}, $leg); }

    ...or maybe it's not so simple, depending on how you want add_leg() to act, which kind of further proves the point. The existence of a relationship between two tables does not dictate (or even suggest, really) anything about how to "correctly" add rows to a related table.

    Oh sure, you can know which columns point to which other columns and other technical details, but semantically, when it comes to adding rows to related tables, a lot is unknown. Is there a limit on the number of related rows that should exist? Should related rows be saved immediately or only saved when the "master" row is saved? Is it even permissible to add rows to a particular related table? There are too many questions for a "safe" add-related-rows method to be implicitly created by default, IMO.

    Anyway, no matter what you do, if you want to support multiple RDBMS-OO mappers on the back-end, you're going to have to make some sort of "adapter" layer to map from your API to the DB layer's API.

    That's a lot of work, and it seems to me that the cost/benefit ratio is hard to justify. I'd say just pick an RDBMS-OO mapper you like (or even none at all, and use straight DBI) and then stick with it. If you really need to tear it out later and replace it, then you can create the required adapter layer in order to maintain your API. But if you never have to do that, then you just saved yourself a lot of work.

      That's a lot of work, and it seems to me that the cost/benefit ratio is hard to justify. I'd say just pick an RDBMS-OO mapper you like (or even none at all, and use straight DBI) and then stick with it. If you really need to tear it out later and replace it, then you can create the required adapter layer in order to maintain your API. But if you never have to do that, then you just saved yourself a lot of work.

      Quite a bit of work. But that's a matter of perspective depending on the situation. If I'm stuck with say for example Class::DBI, I still have to create a layer to seperate it's is-a junk from my top-level module. If I'm already doing the work to do that, the rest is easy.

      As you point out, Rose::DB::Object is a little more polite about things. In the big scheme of things, I'd never need multiple SQL->OOP mappers. My frustration with isa-a and Class::DBI/DBIx::Class has turned into hacking/learning session on using multiple layers or abstracting out the is-a problems. Add 3 parts of Because I Can to that recipe. :-)

Re: A Growing Dislike for SQL-OOP Mappers
by perrin (Chancellor) on Aug 22, 2005 at 18:11 UTC
    I don't see much here that is specific to O/R mappers. What you seem to be saying is that when subclassing things you have to be careful not to conflict with methods in the base class. That's true of anything. If I subclass CGI.pm and want to add a method called param() to my class, it will conflict.

    As for add_to_*, well, you have to commit to some API sometime. I'm not really sure what you are complaining about there. When I don't like the Class::DBI methods, I make my own. If I stop using Class::DBI, I can implement whatever public methods I created using vanilla DBI or Rose::DB or whatever else I happen to switch to.

      When I don't like the Class::DBI methods, I make my own.

      My complaint was that sometimes that's just not possible if the SUPER classes internals call the method you are overriding (as apposed to it's version of that method). I only want the consumer of my class to call my new; not the the internals of the superclass; yet I still want all of the is-a relationship perks; column and setup accessors.

      Updated...

      For example, create a class that is-a Class::DBI and override create. Now call find_or_create(). Bad things happen.

        The problem has nothing to do with SQL-OOP mapping; it's just general OOP. In the end, if you override create and another Class::DBI method (such as find_or_create) calls $self->create, it is perfectly natural that it will call your overriden method. That's what polymorphism is all about.

        Think about it the other way: if it worked the way you want, someone would soon complain that they overrode create and called find_or_create expecting it to call their overriden create, but it didn't.

        If you create an infinite loop or somesuch thing by overriding create, you should reconsider your strategy. You can either override find_or_create as well, or hack your method to keep state so that it breaks out of the loop.

Re: A Growing Dislike for SQL-OOP Mappers
by diotalevi (Canon) on Aug 22, 2005 at 20:29 UTC
    I wonder if a good part of your problem is that you used subclassing at all. Delegation is cleaner and won't invoke these problems. You can use Class::Delegation to make much of the work automatic if you wish.
Re: A Growing Dislike for SQL-OOP Mappers
by jk2addict (Chaplain) on Aug 22, 2005 at 16:23 UTC

    As someone pointed out on use.perl, the real problem is the forcing of an is-a relationship between my object, and it's DB layer. As time goes on, I thik that is-a relationship is more of a bad thing, unless you're really in a hurry to get something done and just want CDBI/DBIC to do everything for you.

Re: A Growing Dislike for SQL-OOP Mappers
by markjugg (Curate) on Aug 23, 2005 at 02:01 UTC
    This sounds like what I ran into when I tried to sub-class HTML::Template.

    The core problem was that something like this existed in HTML::Template:

    # It's own "new()" method was being called internally. HTML::Template->new()

    Call new() on the current package seemed to fix that:

    my $pkg = ref $self $pkg->new();

    Details are in the bug report:

    http://rt.cpan.org/NoAuth/Bug.html?id=14037

Re: A Growing Dislike for SQL-OOP Mappers
by pdcawley (Hermit) on Aug 23, 2005 at 21:30 UTC
    This is the issue that Pixie tries to address. Pixie doesn't mess with the internals of your object. It doesn't require you to implement your object in any particular way. It doesn't need you to write a schema describing your object. It doesn't need you to add a table for every class. It'll save pretty much anything. And if I can't save something, there are hooks so you can transform your object to and from something Pixie can save (and those hooks have reasonably distinctive names, so they shouldn't clash with your existing methods).

    Of course, there are drawbacks. You can't use SQL to find your objects for instance (less of a drawback than a deliberate choice, the plan was to add the capability to do more advanced searching later). And it currently sucks at concurrency (we do have a way forward on fixing that, but it's a matter of summoning up the enthusiasm/tuits). And if you've pulled a lot of objects from the store and mutate only a small proportion of them it'll be rather inefficient at writing things back to the database (this is the killer, for the life of us we can't think of a way of making this work in Perl 5; not without violating the principle of being catholic about what we'll store). And it could be better maintained. But it's worth a look.

    The Perl 6 version will probably prod some serious buttock though.

Re: A Growing Dislike for SQL-OOP Mappers
by markle (Initiate) on Feb 11, 2008 at 22:17 UTC
    Why not just call SUPER::new() at the end of your new() override after doing your filesystem stuff instead of returning? Then you do not have to worry about how the ORM happens to handle things. In your specific example, why would you want to die if the directory exists? Just pass on and call SUPER::new(). Or spell it out as Class::DBI::new(), if you are using multiple bases. Stay out of its way instead?
Re: A Growing Dislike for SQL-OOP Mappers
by jk2addict (Chaplain) on Aug 24, 2005 at 14:58 UTC

    All very good information. I'm glad we don't all agree here. That's means thought is going on. The one thing I've realized is that my gripe about current sql->oop practices in perl is that the current popular mappers assume the stance that the data access object is the same as my public object/interface. That may be true in some cases; not true or desirable in others....and both sides are OK.

      Howdy!

      Interesting...

      I had to write a term paper on an "advanced Java server technology" for a Java programming course. I chose to look at Object-Relational Mapping, and focused on something called SimpleORM.

      Some ORMs try to make persistent objects indistinguishable from non-persistent objects, while others force you to directly acknowledge the difference in the code. SimpleORM did the latter (although you could then put a wrapper/facade around DAOs to hide that down the line. Now, one reason I liked SimpleORM was that it doesn't use XML anywhere, and doesn't try to hide the fact that some classes are different because they have a backing store for persistence.

      yours,
      Michael

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlmeditation [id://485692]
Approved by Roger
Front-paged by Tanalis
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others studying the Monastery: (3)
As of 2024-12-02 17:47 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found