[Koha-devel] Fwd: [RFC: DBIx::Class 2/3] DBix::Class - moving C4::ClassSource over to using DBIx::Class

[Adam Thick]

Hi All,
Andrew thought it would be a good idea to forward this onto the list  
to get some discussion going about the use of DBIx::Class in Koha. I  
sent him an email about it's use, I think it's a great idea!

Cheers,
Adam T.

Begin forwarded message:

> From: "Andrew Moore" <andrew.moore at liblime.com>
> Date: 31 October 2008 12:46:06 AM
> To: "Adam Thick" <adam.thick at strategicdata.com.au>
> Subject: Re: [Koha-devel] [RFC: DBIx::Class 2/3] DBix::Class -  
> moving C4::ClassSource over to using DBIx::Class
>
> Hi Adam -
>
> Thanks for taking the time to look over the patch and make your
> comments. I'm glad that you agree that something like this would help
> Koha. I'm hopeful that I can get it refined enough to get it accepted.
> I think you and I agree on a lot of the points you brought up. Let me
> hit a few of them one at a time:
>
> On Wed, Oct 29, 2008 at 7:47 PM, Adam Thick
> <adam.thick at strategicdata.com.au> wrote:
>> I also think it would be a really good fit for Koha to
>> standardise the calls to the database and ensure the SQL is  
>> sanitised.
>> I'd love to one day see Koha as a set of objects, like Koha::Member  
>> or
>> Koha::Biblio, where Koha has attributes that can be handed round to  
>> each
>> object, and each object with it's own set of attributes and methods  
>> to get
>> things done, ah but one day!
>
> Yes, I'd like to make many of the C4::* packages into classes of
> objects, and probably inherit properties from a base class, too. I
> think that's a step that will be easier to take after this one,
> though. Joe Atzburger and I have been playing with a few different
> possibilities, so stay tuned for proposals along those lines. If you
> have an example implementation of this, I encourage you to send it
> along to the koha-devel list to gauge the community response.
>
>> I'd recommend creating the DBIx::Class schema with detailed column
>> information and inflator/deflators for things like dates and such  
>> as a start
>> too. This is time consuming to go back and add later, and having  
>> this kind
>> of information helps create SQL from your schema and provide  
>> upgrade scripts
>> :)
>
> That's a good point. In the improvements that I've made since Friday,
> I'm using the Schema::Loader to make files that represent each of the
> tables, instead of loading them to memory each time. In them, I'm
> inflating dates, but nothing else. What else should I be inflating?
>
>>> my $sources = GetClassSources();
>>> @@ -91,13 +94,11 @@ foreach my $cn_source (sort keys %$sources) {
>>> sub GetClassSources {
>>>
>>>   my %class_sources = ();
>>> -    my $dbh = C4::Context->dbh;
>>> -    my $sth = $dbh->prepare_cached("SELECT * FROM  
>>> `class_sources`");
>>> -    $sth->execute();
>>> -    while (my $source = $sth->fetchrow_hashref) {
>>> -        $class_sources{ $source->{'cn_source'} } = $source;
>>> +
>>> +    my @class_sources = $schema->resultset('ClassSources')- 
>>> >search();
>>> +    foreach my $row ( @class_sources ) {
>>> +        $class_sources{ $row->get_column('cn_source') } = {
>>> $row->get_columns };
>>>   }
>>> -    $sth->finish();
>>>
>>>   return \%class_sources;
>>>
>>
>> I found rather than using arrays to store the search in it was a  
>> bit faster
>> ( cause your not loading up an array of data ) and syntactically  
>> prettier to
>> get a result set like this
>>
>> my $class_sources = $schema->resultset('ClassSources')->search();
>> while ( my $row = $class_sources->next() ) {
>>       $class_sources->{$row->cn_source} = $row->get_columns
>>  #not sure if this can be called on a cursor or not, might want to  
>> look at
>> something like
>> http://search.cpan.org/~ash/DBIx-Class-0.08010/lib/DBIx/Class/ResultClass/HashRefInflator.pm
>
> I'm not a DBIC expert, but I tried sometime similar to that at first
> and then switched to my method for some reason. I think it was so that
> I could get ahold of the ->columns call. I was probably overlooking a
> call like the one you show. I'm sure that we'll get better at using
> DBIC as we go along. I'll depend on folks like you to help us use it
> better.
>
>> I am really, really happy that database abstraction is being  
>> considered for
>> Koha. It will mean the code base will be easier to maintain and can
>> potentially make database upgrading easier too. I'd like to know if  
>> you are
>> considering a MVC approach to Koha ( in the long run :) I've been  
>> running
>> with Catalyst for a while now too and find that it's great for  
>> separation
>> between Controller, Model and View and definitely makes maintenance  
>> and
>> coding easier. Separating out the code like this also means we can
>> re-purpose elements like the Model to perform other tasks like  
>> providing a
>> webservice :) or cron scripts etc...
>
> I'd like to switch to something like Catalyst at some point, but
> that's a pretty long-term goal. If you have any ways to help move in
> that direction, I encourage you to share them with us. The roadmap
> that I'm working on kind of goes along the lines of:
> * abstract out the SQL using DBIC or maybe SQL::Abstract or  
> DBIx::Abstract
> * improve the database upgrade tools by relying on something like
> SQL::Translator and DBIx::Class::Schema::Versioned
> * start making packages into classes. Maybe use something like Moose?
> * Move to something like Catalyst or whatever is good by this point  
> in time.
>
> I'm glad that you and I seem to be in agreement on the kinds of things
> that would make Koha easier to use, maintain, and extend. Please don't
> hesitate to contribute your thoughts (or patches!) to the koha-devel
> list.
>
> Thanks!
> -Andy

Adam Thick

Strategic Data Pty Ltd

Ph :   +61 (3) 9340-9000
Fax:   +61 (3) 9348-2015
Mob:   0404 032 763
Email: Adam.Thick at StrategicData.com.au
Web:   http://www.StrategicData.com.au/
Post:  P.O. Box 1166
        	   Carlton, VIC  3053
            Australia






-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/koha-devel/attachments/20081105/da0eb360/attachment-0003.htm>