[Dbix-class] Inflation and Deflation

Jose Luis Martinez jlmartinez-lists-dbix at capside.com
Thu Jun 11 15:05:11 GMT 2009


Matt S Trout escribió:
> 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.

No problem. Sorry for the namespace invasion. I've already scheduled the 
deletion in PAUSE.

> 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.

+1. Since I started out with a JSON inflator for one of my projects, I 
extended the behaviour to other serializers, thinking of usefulness for 
other people (not so much in elegance of the code :p).

> 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.

Let's see if you like it better like this:

method select_freezer
   - selects the get_xxx_freezer to call depending on the properties of 
the column (provided just in case someone wants to override the method 
for selecting freezers).
   - get_bounded_column_freezer and unbounded_freezer get transformed 
into $self->get_bounded_column_freezer, and $self->unbounded_freezer. 
Both will return subs that let the column get unfrozen. Are you OK with 
returning subs?

> 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 ...

Didn't want to document them, since I consider they are internal to the 
working of the inflators. maybe I should change them to sub 
_get_xxx_freezer? Maybe document select_freezer so people can override 
that one?

>> 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.

s/DBIx/DBIC/ (fast typing. null review).

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

That's what I was looking for... 0.01 exposes an idea. The feedback gets 
done what the community wants/needs/thinks (not just what I 
want/need/think :))

> 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? :)
> 
+1 for a SVN repo.

Thanks for the opinion and the pointers,

Jose Luis Martinez
jlmartinez at capside.com



More information about the DBIx-Class mailing list