[Dbix-class] Inflation and Deflation

[Matt S Trout]

On Mon, Jun 08, 2009 at 11:52:47PM +0200, Jose Luis Martinez wrote:
> Hi,
> 
>   I've uploaded DBIx-Class-Inflator-Serializers to CPAN 
> (http://search.cpan.org/~jlmartin/DBIx-Class-Inflator-Serializers-0.01/). 

Ok. Great idea. Glad you made the effort to write and release it. Now for
the part where I tear the details to pieces, please hold onto your chair.

First, please delete the current copy from CPAN, it's in the wrong namespace.

Column inflators go in DBIx::Class::InflateColumn, not ::Inflator:: - stealing
random DBIC subnamespaces when there's already an accepted convention isn't
very polite and perhaps more importantly will make your code much harder to
find on CPAN.

Second, the architecture's silly. What you wanted to write was a single
DBIx::Class::InflateColumn::Serializer that uses a driver model like
DBIx::Class::EncodedColumn and DBIx::Class::UUIDColumns do.

It also seems like if you do that then the length check could be generalised.

Third, your code is full of things like -

    if (defined $info->{'size'}){
       $freezer = get_bounded_column_freezer($info->{'size'});
    } else {
       $freezer = \&unbounded_freezer;
    }

which is catastrophically bad since you've now added methods to the class
that can't actually be called as methods because they're really subs, and
can't be overriden by subclasses either.

Within OO code, please only use method calls and document them so people
can override them - otherwise your code isn't usefully pre-packaged because
people will have to copy and paste it in order to extend it ...

> Hope you like it. I'd love to get feedback from the DBIx community to 
> get this package to a 1.00 state.

There's no such thing as "the DBIx community". DBIx is the namespace for
DBI extensions, DBIx::Class is one project in that namespace.

Just like DBix::Class::InflateColumn:: is the namespace for DBIx::Class
inflators, and there are many projects in that namespace.

Plus yours in the wrong one until you delete it and fix it :)

All that said, I think the -idea- is great - just the name, architecture
and implementation need some work :)

I look forward to looking at a tarball or repository of a rewritten version
so we can try and give feedback on the new one -before- it goes to CPAN ...

(also, I'd be happy to see this as a "DBIC team supported project" - in
which case could we offer you subversion or git hosting for it? :)

-- 
        Matt S Trout         Catalyst and DBIx::Class consultancy with a clue
     Technical Director      and a commit bit: http://shadowcat.co.uk/catalyst/
 Shadowcat Systems Limited
  mst (@) shadowcat.co.uk        http://shadowcat.co.uk/blog/matt-s-trout/