As discussed in #koha, I think we are not using DBIC as well as we could. Right now we are only using it to replace hand written SQL queries. I would propose the following: 1) Allow find and simple searches in pl. 2) If a search is used more than once, it should be a ResultSet method 3) If a subroutine operates on a single table row, it should be a Result method 4) If a subroutine operates on a single table, it should be a ResultSet method 5) Any operation that works on tables linked by key constraints should take advantage of those DBIC relationships and also In this way we can do things like: $borrower->is_debarred or $borrower->has_overdues or @borrowers = $borrower->guarantees() Any search can be a method in the ResultSet such as $borrower->issue()->overdue_issues(); or my @records_with_holds = $schema->resultset('Biblio')->with_holds() I think in the long run this will make or code far more readable, testable, and efficient. What do you think? Kyle http://www.kylehall.info ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com )
Hi Kyle: Could you speak more to what a ResultSet method would be? From what I’ve seen Koha::Schema::* just has Results in it. Would we need to define custom ResultSets as well to get this kind of functionality? Overall, this sounds good to me. It would be nice to put more of the “database” code into one place, and have more of the “logic” or processing elsewhere. Like you say, more readable, testable, efficient. David Cook Systems Librarian Prosentient Systems 72/330 Wattle St, Ultimo, NSW 2007 From: koha-devel-bounces@lists.koha-community.org [mailto:koha-devel-bounces@lists.koha-community.org] On Behalf Of Kyle Hall Sent: Wednesday, 10 September 2014 3:22 AM To: Koha Devel Subject: [Koha-devel] Koha and DBIC As discussed in #koha, I think we are not using DBIC as well as we could. Right now we are only using it to replace hand written SQL queries. I would propose the following: 1) Allow find and simple searches in pl. 2) If a search is used more than once, it should be a ResultSet method 3) If a subroutine operates on a single table row, it should be a Result method 4) If a subroutine operates on a single table, it should be a ResultSet method 5) Any operation that works on tables linked by key constraints should take advantage of those DBIC relationships and also In this way we can do things like: $borrower->is_debarred or $borrower->has_overdues or @borrowers = $borrower->guarantees() Any search can be a method in the ResultSet such as $borrower->issue()->overdue_issues(); or my @records_with_holds = $schema->resultset('Biblio')->with_holds() I think in the long run this will make or code far more readable, testable, and efficient. What do you think? Kyle http://www.kylehall.info ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com )
And what about deploying instead of kohastructure.sql ? Same for DBIx statements in updatedatabase.. The current process was meant to be temporary (adding SQL in kohastructure and updatedatabase and having the RM update the DBIx scheme). ________________________________ Van: koha-devel-bounces@lists.koha-community.org [koha-devel-bounces@lists.koha-community.org] namens David Cook [dcook@prosentient.com.au] Verzonden: woensdag 10 september 2014 8:26 Aan: 'Kyle Hall'; 'Koha Devel' Onderwerp: Re: [Koha-devel] Koha and DBIC Hi Kyle: Could you speak more to what a ResultSet method would be? From what I’ve seen Koha::Schema::* just has Results in it. Would we need to define custom ResultSets as well to get this kind of functionality? Overall, this sounds good to me. It would be nice to put more of the “database” code into one place, and have more of the “logic” or processing elsewhere. Like you say, more readable, testable, efficient. David Cook Systems Librarian Prosentient Systems 72/330 Wattle St, Ultimo, NSW 2007 From: koha-devel-bounces@lists.koha-community.org [mailto:koha-devel-bounces@lists.koha-community.org] On Behalf Of Kyle Hall Sent: Wednesday, 10 September 2014 3:22 AM To: Koha Devel Subject: [Koha-devel] Koha and DBIC As discussed in #koha, I think we are not using DBIC as well as we could. Right now we are only using it to replace hand written SQL queries. I would propose the following: 1) Allow find and simple searches in pl. 2) If a search is used more than once, it should be a ResultSet method 3) If a subroutine operates on a single table row, it should be a Result method 4) If a subroutine operates on a single table, it should be a ResultSet method 5) Any operation that works on tables linked by key constraints should take advantage of those DBIC relationships and also In this way we can do things like: $borrower->is_debarred or $borrower->has_overdues or @borrowers = $borrower->guarantees() Any search can be a method in the ResultSet such as $borrower->issue()->overdue_issues(); or my @records_with_holds = $schema->resultset('Biblio')->with_holds() I think in the long run this will make or code far more readable, testable, and efficient. What do you think? Kyle http://www.kylehall.info ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com )
That is certainly a possibility. There are a few options with DBIC. However, I ended up abandoning all of them for something custom with Libki. We may have better results collectively than I did alone. Kyle http://www.kylehall.info ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com ) On Wed, Sep 10, 2014 at 2:39 AM, Marcel de Rooy <M.de.Rooy@rijksmuseum.nl> wrote:
And what about deploying instead of kohastructure.sql ? Same for DBIx statements in updatedatabase.. The current process was meant to be temporary (adding SQL in kohastructure and updatedatabase and having the RM update the DBIx scheme).
------------------------------ *Van:* koha-devel-bounces@lists.koha-community.org [ koha-devel-bounces@lists.koha-community.org] namens David Cook [ dcook@prosentient.com.au] *Verzonden:* woensdag 10 september 2014 8:26 *Aan:* 'Kyle Hall'; 'Koha Devel' *Onderwerp:* Re: [Koha-devel] Koha and DBIC
Hi Kyle:
Could you speak more to what a ResultSet method would be? From what I’ve seen Koha::Schema::* just has Results in it. Would we need to define custom ResultSets as well to get this kind of functionality?
Overall, this sounds good to me. It would be nice to put more of the “database” code into one place, and have more of the “logic” or processing elsewhere. Like you say, more readable, testable, efficient.
David Cook
Systems Librarian
Prosentient Systems
72/330 Wattle St, Ultimo, NSW 2007
*From:* koha-devel-bounces@lists.koha-community.org [mailto: koha-devel-bounces@lists.koha-community.org] *On Behalf Of *Kyle Hall *Sent:* Wednesday, 10 September 2014 3:22 AM *To:* Koha Devel *Subject:* [Koha-devel] Koha and DBIC
As discussed in #koha, I think we are not using DBIC as well as we could. Right now we are only using it to replace hand written SQL queries. I would propose the following:
1) Allow find and simple searches in pl.
2) If a search is used more than once, it should be a ResultSet method
3) If a subroutine operates on a single table row, it should be a Result method
4) If a subroutine operates on a single table, it should be a ResultSet method
5) Any operation that works on tables linked by key constraints should take advantage of those DBIC relationships and also
In this way we can do things like:
$borrower->is_debarred
or
$borrower->has_overdues
or
@borrowers = $borrower->guarantees()
Any search can be a method in the ResultSet such as
$borrower->issue()->overdue_issues();
or
my @records_with_holds = $schema->resultset('Biblio')->with_holds()
I think in the long run this will make or code far more readable, testable, and efficient.
What do you think?
Kyle
ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com )
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
We need to create the ResultSets ourselves. We should just be able to dump them in Koha/Schema/ResultSet. I'll work on a proof of concept. Here is an example: http://blogs.perl.org/users/ovid/2014/04/custom-dbixclass-resultsets.html I agree. Kyle http://www.kylehall.info ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com ) On Wed, Sep 10, 2014 at 2:26 AM, David Cook <dcook@prosentient.com.au> wrote:
Hi Kyle:
Could you speak more to what a ResultSet method would be? From what I’ve seen Koha::Schema::* just has Results in it. Would we need to define custom ResultSets as well to get this kind of functionality?
Overall, this sounds good to me. It would be nice to put more of the “database” code into one place, and have more of the “logic” or processing elsewhere. Like you say, more readable, testable, efficient.
David Cook
Systems Librarian
Prosentient Systems
72/330 Wattle St, Ultimo, NSW 2007
*From:* koha-devel-bounces@lists.koha-community.org [mailto: koha-devel-bounces@lists.koha-community.org] *On Behalf Of *Kyle Hall *Sent:* Wednesday, 10 September 2014 3:22 AM *To:* Koha Devel *Subject:* [Koha-devel] Koha and DBIC
As discussed in #koha, I think we are not using DBIC as well as we could. Right now we are only using it to replace hand written SQL queries. I would propose the following:
1) Allow find and simple searches in pl.
2) If a search is used more than once, it should be a ResultSet method
3) If a subroutine operates on a single table row, it should be a Result method
4) If a subroutine operates on a single table, it should be a ResultSet method
5) Any operation that works on tables linked by key constraints should take advantage of those DBIC relationships and also
In this way we can do things like:
$borrower->is_debarred
or
$borrower->has_overdues
or
@borrowers = $borrower->guarantees()
Any search can be a method in the ResultSet such as
$borrower->issue()->overdue_issues();
or
my @records_with_holds = $schema->resultset('Biblio')->with_holds()
I think in the long run this will make or code far more readable, testable, and efficient.
What do you think?
Kyle
ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com )
Hi all, IMO a ResultSet should not be instantiate in a pl script for some reasons (I already listed yesterday, nothing new :)) - should be unit tested - easier to maintain - easier to reuse But I don't think it's a big deal. I think someone (don't remember who) explained it is not a good idea to add too much changes to the ResultSet class. I tend to agree. In your example: $borrower->is_debarred Where do you instantiate $borrower? And where is defined is_debarred? I would write something like I have written for Koha::Acquisition::Order (bug 12830) package Koha::Borrower; use Koha::Schema; use base qw( Class::Accessor ); sub fetch { my ( $class, $borrowernumber ) = @_; my $borrower = Koha::Database->new->schema->resultset('Borrower')->find($borrowernumber, {result_class => 'DBIx::Class::ResultClass::HashRefInflator' } ); return $class->new( $borrower ); } sub is_debarred { my ( $self ) = @_; my $debarred_date = $self->{debarred}; return $debarred_date > $today; } in the pl file: my $borrower = Koha::Borrower->fetch( $borrowernumber ); The limitation in this implementation is that $borrower is not a DBIC object and we would like to get rid of HashRefInflator. Please comment this part of code and share yours! Cheers, Jonathan 2014-09-09 19:21 GMT+02:00 Kyle Hall <kyle.m.hall@gmail.com>:
As discussed in #koha, I think we are not using DBIC as well as we could. Right now we are only using it to replace hand written SQL queries. I would propose the following:
1) Allow find and simple searches in pl. 2) If a search is used more than once, it should be a ResultSet method 3) If a subroutine operates on a single table row, it should be a Result method 4) If a subroutine operates on a single table, it should be a ResultSet method 5) Any operation that works on tables linked by key constraints should take advantage of those DBIC relationships and also
In this way we can do things like: $borrower->is_debarred or $borrower->has_overdues or @borrowers = $borrower->guarantees()
Any search can be a method in the ResultSet such as $borrower->issue()->overdue_issues(); or my @records_with_holds = $schema->resultset('Biblio')->with_holds()
I think in the long run this will make or code far more readable, testable, and efficient.
What do you think?
Kyle
http://www.kylehall.info ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com )
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
On Wed, Sep 10, 2014 at 5:00 AM, Jonathan Druart < jonathan.druart@biblibre.com> wrote:
Hi all,
IMO a ResultSet should not be instantiate in a pl script for some reasons (I already listed yesterday, nothing new :)) - should be unit tested - easier to maintain - easier to reuse
But I don't think it's a big deal.
I agree with all these principles, and none of them conflict with what I am proposing. Result and ResultSet methods are no different than C4/Koha methods and subroutines.
I think someone (don't remember who) explained it is not a good idea to add too much changes to the ResultSet class. I tend to agree.
In your example: $borrower->is_debarred Where do you instantiate $borrower? And where is defined is_debarred?
Most likely you would use find(), single() or search(). In this example we could easily swith from Koha::Borrower::Debarments::IsDebarred to Koha::Schema::ResultSet::Borrower::is_debarred. There is no functional difference, and it makes more sense in the long run. I agree that any search that is even mildly complex and used in multiple places should live in the ResultSet. I'm not saying we should use search() everyone where in pl files, I'm just against a hard and fast rule forbidding it which will cause developers to jump through hoops that don't need to exist.
I would write something like I have written for Koha::Acquisition::Order (bug 12830)
package Koha::Borrower; use Koha::Schema; use base qw( Class::Accessor );
sub fetch { my ( $class, $borrowernumber ) = @_; my $borrower = Koha::Database->new->schema->resultset('Borrower')->find($borrowernumber, {result_class => 'DBIx::Class::ResultClass::HashRefInflator' } ); return $class->new( $borrower ); }
sub is_debarred { my ( $self ) = @_; my $debarred_date = $self->{debarred}; return $debarred_date > $today; }
in the pl file: my $borrower = Koha::Borrower->fetch( $borrowernumber );
The limitation in this implementation is that $borrower is not a DBIC object and we would like to get rid of HashRefInflator.
Please comment this part of code and share yours!
Can you see how this gives no advantage, but simply wraps the useful code in another layer of abstraction? There simply is no advantage to doing this. In the long run this type of thinking will make things more complicated and more error prone than they need to be. Instead of all this code we simply need to add one method to Result::Borrower. Here is the gist of it: sub is_debarred { my ( $self ) = @_; return $self->debarred_date() > dt_from_string(); } Think about all the overhead and complexity that is removed by this variation. Kyle
2014-09-11 2:39 GMT+02:00 Kyle Hall <kyle.m.hall@gmail.com>:
On Wed, Sep 10, 2014 at 5:00 AM, Jonathan Druart <jonathan.druart@biblibre.com> wrote:
Hi all,
IMO a ResultSet should not be instantiate in a pl script for some reasons (I already listed yesterday, nothing new :)) - should be unit tested - easier to maintain - easier to reuse
But I don't think it's a big deal.
I agree with all these principles, and none of them conflict with what I am proposing. Result and ResultSet methods are no different than C4/Koha methods and subroutines.
Yes they are, as Robin said, It is an API for the DB, not for the functional job.
Please comment this part of code and share yours!
Can you see how this gives no advantage, but simply wraps the useful code in another layer of abstraction? There simply is no advantage to doing this. In the long run this type of thinking will make things more complicated and more error prone than they need to be.
The advantage is to share responsabilities, which is already a lot.
Instead of all this code we simply need to add one method to Result::Borrower. Here is the gist of it:
sub is_debarred { my ( $self ) = @_; return $self->debarred_date() > dt_from_string(); }
Think about all the overhead and complexity that is removed by this variation.
I don't understand where is the complexity. We need Koha::Module packages. We cannot put all our code into Koha::Schema::Result::Module. Why everyone is afraid of abstraction layers?
On Thu, Sep 11, 2014 at 3:31 AM, Jonathan Druart < jonathan.druart@biblibre.com> wrote:
2014-09-11 2:39 GMT+02:00 Kyle Hall <kyle.m.hall@gmail.com>:
On Wed, Sep 10, 2014 at 5:00 AM, Jonathan Druart <jonathan.druart@biblibre.com> wrote:
Hi all,
IMO a ResultSet should not be instantiate in a pl script for some reasons (I already listed yesterday, nothing new :)) - should be unit tested - easier to maintain - easier to reuse
But I don't think it's a big deal.
I agree with all these principles, and none of them conflict with what I am proposing. Result and ResultSet methods are no different than C4/Koha methods and subroutines.
Yes they are, as Robin said, It is an API for the DB, not for the functional job.
Again, only if we treat it that way. We can leverage DBIC to add formality and commonality to our OO code where it's always been bespoke implementations previously.
Please comment this part of code and share yours!
Can you see how this gives no advantage, but simply wraps the useful code in another layer of abstraction? There simply is no advantage to doing this. In the long run this type of thinking will make things more complicated and more error prone than they need to be.
The advantage is to share responsabilities, which is already a lot.
I don't understand your comment.
Instead of all this code we simply need to add one method to Result::Borrower. Here is the gist of it:
sub is_debarred { my ( $self ) = @_; return $self->debarred_date() > dt_from_string(); }
Think about all the overhead and complexity that is removed by this variation.
I don't understand where is the complexity. We need Koha::Module packages. We cannot put all our code into Koha::Schema::Result::Module. Why everyone is afraid of abstraction layers?
We *don't* need Koha::Module packages for everything! My point is that DBIC provides us with an abstraction layer already! Wrapping a DBIC class in another class provides no advantage and just obfuscates our code, increases hardware requirements, and leaves more room to introduce bugs. If we are just going to use DBIC to avoid writing direct sql queries, we should be using something like Fey instead. We are a collective of developers with many different styles and opinions. We often have very different approaches to how we implement new features. If we go down this path of wrapping DBIC in another abstraction layer, we simply continue down this patch of having many different bespoke implementations. If we use dbic as our abstraction layer, we remove much of that issue. Kyle
2014-09-11 13:29 GMT+02:00 Kyle Hall <kyle.m.hall@gmail.com>:
Can you see how this gives no advantage, but simply wraps the useful code in another layer of abstraction? There simply is no advantage to doing this. In the long run this type of thinking will make things more complicated and more error prone than they need to be.
The advantage is to share responsabilities, which is already a lot.
I don't understand your comment.
Not important, I just repeated myself: the functional job should not be done under the Koha::Schema::Result. It should only contain DB relationships without any intelligence on what happen above.
Instead of all this code we simply need to add one method to Result::Borrower. Here is the gist of it:
sub is_debarred { my ( $self ) = @_; return $self->debarred_date() > dt_from_string(); }
Think about all the overhead and complexity that is removed by this variation.
I don't understand where is the complexity. We need Koha::Module packages. We cannot put all our code into Koha::Schema::Result::Module. Why everyone is afraid of abstraction layers?
We *don't* need Koha::Module packages for everything! My point is that DBIC provides us with an abstraction layer already! Wrapping a DBIC class in another class provides no advantage and just obfuscates our code, increases hardware requirements, and leaves more room to introduce bugs. If we are just going to use DBIC to avoid writing direct sql queries, we should be using something like Fey instead.
Give me examples please: Which "modules" don't need specific packages? IMO, these ones should have their own modules: Order, Supplier, Biblio, Item, Issue, Authorised value, Suggestion, Library, Budget, Fund, Patron.
We are a collective of developers with many different styles and opinions.
I completely agree with that :) That fact is that DBIC has been pushed into Koha 1 year ago and we don't have any plan. I tried several times and my patches have just been rejected. We never had this discussion (I mean a real/constructive discussion), nobody gives examples. I really would like someone shows me what he has in mind, like I did for Koha::Acquisition::[Order|Bookseller].
We often have very different approaches to how we implement new features. If we go down this path of wrapping DBIC in another abstraction layer, we simply continue down this patch of having many different bespoke implementations. If we use dbic as our abstraction layer, we remove much of that issue.
I we reach an agreement, we can do what we want. I hope some of you will be in Cordoba in 3 weeks! :)
I don't understand where is the complexity. We need Koha::Module packages. We cannot put all our code into Koha::Schema::Result::Module. Why everyone is afraid of abstraction layers?
We *don't* need Koha::Module packages for everything! My point is that DBIC provides us with an abstraction layer already! Wrapping a DBIC class in another class provides no advantage and just obfuscates our code, increases hardware requirements, and leaves more room to introduce bugs. If we are just going to use DBIC to avoid writing direct sql queries, we should be using something like Fey instead.
Give me examples please: Which "modules" don't need specific packages? IMO, these ones should have their own modules: Order, Supplier, Biblio, Item, Issue, Authorised value, Suggestion, Library, Budget, Fund, Patron.
Why do you believe these need Koha modules? Each one of these already has an object with methods via DBIC. We can add more methods either for a given Library via Result::Branch, or for a collection of libraries via ResultSet::Branch
We are a collective of developers with many different styles and opinions.
I completely agree with that :) That fact is that DBIC has been pushed into Koha 1 year ago and we don't have any plan. I tried several times and my patches have just been rejected. We never had this discussion (I mean a real/constructive discussion), nobody gives examples. I really would like someone shows me what he has in mind, like I did for Koha::Acquisition::[Order|Bookseller].
I'd really love to get a chance to write something that exemplifies this. So far the only bit has been Result::Item::effective_itemtype()
We often have very different approaches to how we implement new features. If we go down this path of wrapping DBIC in another abstraction layer, we simply continue down this patch of having many different bespoke implementations. If we use dbic as our abstraction layer, we remove much of that issue.
I we reach an agreement, we can do what we want.
I hope some of you will be in Cordoba in 3 weeks! :)
I wish I was going! Unfortunately, with a baby in the house I've made the decision not to attend this year. It was not an easy decision to make, and I'll miss getting to see everyone. I hope any discussions regarding these issues will continue to take place here and on #koha so I can participate! Kyle
2014-09-11 14:54 GMT+02:00 Kyle Hall <kyle.m.hall@gmail.com>:
Give me examples please: Which "modules" don't need specific packages? IMO, these ones should have their own modules: Order, Supplier, Biblio, Item, Issue, Authorised value, Suggestion, Library, Budget, Fund, Patron.
Why do you believe these need Koha modules? Each one of these already has an object with methods via DBIC. We can add more methods either for a given Library via Result::Branch, or for a collection of libraries via ResultSet::Branch
Just because in my feeling, they need their own module. To share responsibilities. This abstraction is not a way to make the logic/architecture complicated, but to simplify it. I am not a fan of 2k packages lines...
I'd really love to get a chance to write something that exemplifies this. So far the only bit has been Result::Item::effective_itemtype()
Just a quick example would give me what you think. With biblio or patron, or whatever. I am waiting for examples for 1 year now.
I we reach an agreement, we can do what we want.
I hope some of you will be in Cordoba in 3 weeks! :)
I wish I was going! Unfortunately, with a baby in the house I've made the decision not to attend this year. It was not an easy decision to make, and I'll miss getting to see everyone. I hope any discussions regarding these issues will continue to take place here and on #koha so I can participate!
Too bad!
Greetings, Fey?! Please, let’s not add more technologies (Haskell that compiles to Javascript) in for geekiness sake. I have been reading this dialogue between Robin and Kyle and thinking to myself, “Why was DBIC added in the first place? Was it not to increase portability (where is that ‘it does not run on Postgresql!’ person) , ‘simplify’ (at the cost of another learning curve) data access, and provide a cleaner abstraction for data access?” So, why was it added? Was it added to be more Object Oriented (OO) so we could use it in the way that is being discussed? So, I’ll reiterate my question again, which I think will bring some clarity to the discussion, what was the point of adding DBIC? GPML, Mark Tompsett P.S. Abstractions are okay, but abstractions upon abstractions are ugly for the initial learning curve. If you want people to join the coding community, multiple levels of abstractions make it more difficult.
On Thu, Sep 11, 2014 at 8:13 AM, Mark Tompsett <mtompset@hotmail.com> wrote:
Greetings,
Fey?! Please, let’s not add more technologies (Haskell that compiles to Javascript) in for geekiness sake.
Heh, not Fay, Fey ; ) http://search.cpan.org/~drolsky/Fey-0.40/lib/Fey.pm
I have been reading this dialogue between Robin and Kyle and thinking to myself, “Why was DBIC added in the first place? Was it not to increase portability (where is that ‘it does not run on Postgresql!’ person) , ‘simplify’ (at the cost of another learning curve) data access, and provide a cleaner abstraction for data access?” So, why was it added? Was it added to be more Object Oriented (OO) so we could use it in the way that is being discussed? So, I’ll reiterate my question again, which I think will bring some clarity to the discussion, what was the point of adding DBIC?
I had always thought it was yes to all those. We discussed the issues in Edinburgh. That's where the idea of adding another layer of abstraction was proposed and later discarded.
P.S. Abstractions are okay, but abstractions upon abstractions are ugly for the initial learning curve. If you want people to join the coding community, multiple levels of abstractions make it more difficult.
Agreed. That is why I'm making these suggestions. Kyle
2014-09-11 14:59 GMT+02:00 Kyle Hall <kyle.m.hall@gmail.com>:
I had always thought it was yes to all those. We discussed the issues in Edinburgh. That's where the idea of adding another layer of abstraction was proposed and later discarded.
I was not in Edinburgh and did know stuffs have been discussed. Nothing is writing somewhere, right? So we need to write it now :)
There should be a page on the wiki somewhere. I recall authoring it. Kyle http://www.kylehall.info ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com ) On Thu, Sep 11, 2014 at 9:34 AM, Jonathan Druart < jonathan.druart@biblibre.com> wrote:
2014-09-11 14:59 GMT+02:00 Kyle Hall <kyle.m.hall@gmail.com>:
I had always thought it was yes to all those. We discussed the issues in Edinburgh. That's where the idea of adding another layer of abstraction was proposed and later discarded.
I was not in Edinburgh and did know stuffs have been discussed. Nothing is writing somewhere, right? So we need to write it now :)
+1 On 09/11/2014 08:13 AM, Mark Tompsett wrote:
Greetings, Fey?! Please, let's not add more technologies (Haskell that compiles to Javascript) in for geekiness sake. I have been reading this dialogue between Robin and Kyle and thinking to myself, "Why was DBIC added in the first place? Was it not to increase portability (where is that 'it does not run on Postgresql!' person) , 'simplify' (at the cost of another learning curve) data access, and provide a cleaner abstraction for data access?" So, why was it added? Was it added to be more Object Oriented (OO) so we could use it in the way that is being discussed? So, I'll reiterate my question again, which I think will bring some clarity to the discussion, what was the point of adding DBIC? GPML, Mark Tompsett P.S. Abstractions are okay, but abstractions upon abstractions are ugly for the initial learning curve. If you want people to join the coding community, multiple levels of abstractions make it more difficult.
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Philippe Blouin, Responsable du développement informatique Tél. : (888) 604-2627 philippe.blouin@inLibro.com <mailto:philippe.blouin@inLibro.com> inLibro | pour esprit libre | www.inLibro.com <http://www.inLibro.com>
On Thu, Sep 11, 2014 at 08:13:31AM -0400, Mark Tompsett wrote:
I’ll reiterate my question again, which I think will bring some clarity to the discussion, what was the point of adding DBIC?
One reason implicit was so that we could benefit from the work that's gone into it rather than re-inventing bits of it with varying success as we have done repeatedly and not too consistently. C. -- Colin Campbell Chief Software Engineer, PTFS Europe Limited Content Management and Library Solutions +44 (0) 800 756 6803 (phone) +44 (0) 7759 633626 (mobile) colin.campbell@ptfs-europe.com skype: colin_campbell2 http://www.ptfs-europe.com
Hey, there is a project for Haskell -> Perl using Fay. On Thu, Sep 11, 2014 at 9:13 AM, Mark Tompsett <mtompset@hotmail.com> wrote:
Greetings,
Fey?! Please, let’s not add more technologies (Haskell that compiles to Javascript) in for geekiness sake. I have been reading this dialogue between Robin and Kyle and thinking to myself, “Why was DBIC added in the first place? Was it not to increase portability (where is that ‘it does not run on Postgresql!’ person) , ‘simplify’ (at the cost of another learning curve) data access, and provide a cleaner abstraction for data access?” So, why was it added? Was it added to be more Object Oriented (OO) so we could use it in the way that is being discussed? So, I’ll reiterate my question again, which I think will bring some clarity to the discussion, what was the point of adding DBIC?
GPML, Mark Tompsett P.S. Abstractions are okay, but abstractions upon abstractions are ugly for the initial learning curve. If you want people to join the coding community, multiple levels of abstractions make it more difficult.
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Tomás Cohen Arazi Prosecretaría de Informática Universidad Nacional de Córdoba ✆ +54 351 5353750 ext 13168 GPG: B76C 6E7C 2D80 551A C765 E225 0A27 2EA1 B2F3 C15F
Hi Kyle, Il 09/09/2014 19:21, Kyle Hall ha scritto:
1) Allow find and simple searches in pl. 2) If a search is used more than once, it should be a ResultSet method 3) If a subroutine operates on a single table row, it should be a Result method 4) If a subroutine operates on a single table, it should be a ResultSet method 5) Any operation that works on tables linked by key constraints should take advantage of those DBIC relationships and also
to undestand the proposal is better to have a complete script with this points. Do you have written a patch on bug in this way ? Bye Zeno Tajoli -- Dr. Zeno Tajoli Soluzioni per la Ricerca Istituzionale - Automazione Biblioteche z.tajoli@cineca.it fax +39 02 2135520 CINECA - Sede operativa di Segrate
I'll work on a proof of concept. In the mean time here are a couple examples from another project I work on: http://sourceforge.net/p/bizwerks/bizwerks/ci/master/tree/lib/BizWerks/Schem... http://sourceforge.net/p/bizwerks/bizwerks/ci/master/tree/lib/BizWerks/Schem... http://www.kylehall.info ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com ) On Wed, Sep 10, 2014 at 5:07 AM, Zeno Tajoli <z.tajoli@cineca.it> wrote:
Hi Kyle,
Il 09/09/2014 19:21, Kyle Hall ha scritto:
1) Allow find and simple searches in pl. 2) If a search is used more than once, it should be a ResultSet method 3) If a subroutine operates on a single table row, it should be a Result method 4) If a subroutine operates on a single table, it should be a ResultSet method 5) Any operation that works on tables linked by key constraints should take advantage of those DBIC relationships and also
to undestand the proposal is better to have a complete script with this points. Do you have written a patch on bug in this way ?
Bye Zeno Tajoli -- Dr. Zeno Tajoli Soluzioni per la Ricerca Istituzionale - Automazione Biblioteche z.tajoli@cineca.it fax +39 02 2135520 CINECA - Sede operativa di Segrate
Kyle Hall schreef op di 09-09-2014 om 13:21 [-0400]:
1) Allow find and simple searches in pl.
IMO, this is not a good idea. You're coupling database schema with code that's closer to display. All access should be via an API into the C4:: or Koha:: namespace, and the modules there are the only ones that know/care how the database is actually laid out. This means that if we want to change how things are stored[0], only one module needs to know about the change, and it can still present the same interface to code. The moment that you have more than one place talking to a fairly low-level system like the database, you are going to have problems (e.g. the issues in the current system where so many modules touch the account code that refactoring it is a huge nightmare.) [0] from fixing badly named columns/tables, to adding caching, to refactoring the schema to make it more efficient, to not using the database at all for some reason. -- Robin Sheat Catalyst IT Ltd. ✆ +64 4 803 2204 GPG: 5FA7 4B49 1E4D CAA4 4C38 8505 77F5 B724 F871 3BDF
1) Allow find and simple searches in pl.
IMO, this is not a good idea. You're coupling database schema with code that's closer to display.
It's funny, I've been trying to think of an example search to justify this and I can't really think of one where we can't use existing Result relationships to get our data. Maybe find() and single() are really all we need from pl files, but I don't think a hard and fast rule against search() is a good idea.
All access should be via an API into the C4:: or Koha:: namespace, and the modules there are the only ones that know/care how the database is actually laid out. This means that if we want to change how things are stored[0], only one module needs to know about the change, and it can still present the same interface to code.
Result and ResultSet are part of the Koha namespace. I also feel that you aren't quite correct about how one modules need to know the change a present the same interface. I can't count the number of times I've added a new parameter to an existing subroutine and had to update every function call in many perl files. Now that hashref's are more common, it's become less of a problem but it's still a problem that exists. I just don't think this is a entirely valid point. Remember, Result and ResultSet methods are no different than methods anywhere else in Koha and C4.
The moment that you have more than one place talking to a fairly low-level system like the database, you are going to have problems (e.g. the issues in the current system where so many modules touch the account code that refactoring it is a huge nightmare.)
Exactly. If you look at my accounts rewrite you'll see I didn't try to shoehorn everything into Result and ResultSet. I used a Koha module that uses Results and ResultSets in a sane fashion. Kyle
-- Robin Sheat Catalyst IT Ltd. ✆ +64 4 803 2204 GPG: 5FA7 4B49 1E4D CAA4 4C38 8505 77F5 B724 F871 3BDF
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
Kyle Hall schreef op wo 10-09-2014 om 20:51 [-0400]:
It's funny, I've been trying to think of an example search to justify this and I can't really think of one where we can't use existing Result relationships to get our data. Maybe find() and single() are really all we need from pl files, but I don't think a hard and fast rule against search() is a good idea.
We'd still be coupling to the data storage schema, which is bad. It's exactly the same logic that says not to have SQL in the .pl files.
Result and ResultSet are part of the Koha namespace. I also feel that
Only in the sense that they have Koha:: in the name. They are an API to the database, and are generated directly from the database schema. They are not an API that is designed to provide logical access to whatever it is.
you aren't quite correct about how one modules need to know the change a present the same interface. I can't count the number of times I've added a new parameter to an existing subroutine and had to update every function call in many perl files. Now that hashref's are more common, it's become less of a problem but it's still a problem that exists.
"It's been broken in the past" isn't a justification for "so we should keep it broken." As we go to a more OO and thought out method of writing things (again, especially with the Koha:: stuff), we are moving away from "here's a hashref with the fields from the biblio table" to "here's an object that represents a biblio, with accessors and modifiers." We want more of this, and less things going and talking to the database themselves.
I just don't think this is a entirely valid point. Remember, Result and ResultSet methods are no different than methods anywhere else in Koha and C4.
They are totally different. They are much closer to an abstraction of the database, compared to an abstraction of whatever it is the data is representing. And the code at the .pl level should be working with abstractions of data, e.g. "a reserve", or "a biblio with attached items", not "a set of hashrefs containing a biblio, a biblioitem, and the associated item records." This latter is what Result/ResultSet represent. In the ES code, I've been keeping this in mind. For example, I can go (from memory): my $it = Koha::Biblio->get_iterator; while (my $bib = $it->next) { add_to_index($bib); } which means I don't have to care how they're stored. It also meant that it took very little work to refactor it to handle authorities, as everything had the same style of API. ResultSet stuff happened in the modules, but none of it was exposed to the script that was using those modules.
Exactly. If you look at my accounts rewrite you'll see I didn't try to shoehorn everything into Result and ResultSet. I used a Koha module that uses Results and ResultSets in a sane fashion.
And that's fine. Just not at the .pl level. The job of modules is to put a sensible abstraction between the storage layer and the user interface layer. As soon as UI related code can touch the storage layer, you're violating that principle and causing maintenance headaches for the next person who comes along.
-- Robin Sheat Catalyst IT Ltd. ✆ +64 4 803 2204 GPG: 5FA7 4B49 1E4D CAA4 4C38 8505 77F5 B724 F871 3BDF
On Wed, Sep 10, 2014 at 9:19 PM, Robin Sheat <robin@catalyst.net.nz> wrote:
Kyle Hall schreef op wo 10-09-2014 om 20:51 [-0400]:
It's funny, I've been trying to think of an example search to justify this and I can't really think of one where we can't use existing Result relationships to get our data. Maybe find() and single() are really all we need from pl files, but I don't think a hard and fast rule against search() is a good idea.
We'd still be coupling to the data storage schema, which is bad. It's exactly the same logic that says not to have SQL in the .pl files.
I'm simply saying it doesn't seem that something like search( borrowernumber => $borrowernumber ) should be abstracted away.
Result and ResultSet are part of the Koha namespace. I also feel that
Only in the sense that they have Koha:: in the name. They are an API to the database, and are generated directly from the database schema. They are not an API that is designed to provide logical access to whatever it is.
We can always re-engineer our Result and ResultSet's to use any theoretical future construct. We needn't use another layer of API's.
you aren't quite correct about how one modules need to know the change a present the same interface. I can't count the number of times I've added a new parameter to an existing subroutine and had to update every function call in many perl files. Now that hashref's are more common, it's become less of a problem but it's still a problem that exists.
"It's been broken in the past" isn't a justification for "so we should keep it broken." As we go to a more OO and thought out method of writing things (again, especially with the Koha:: stuff), we are moving away from "here's a hashref with the fields from the biblio table" to "here's an object that represents a biblio, with accessors and modifiers." We want more of this, and less things going and talking to the database themselves.
I agree. DBIC *provides* objects that represent a biblio with accessors and modifiers.
I just don't think this is a entirely valid point. Remember, Result and ResultSet methods are no different than methods anywhere else in Koha and C4.
They are totally different. They are much closer to an abstraction of the database, compared to an abstraction of whatever it is the data is representing. And the code at the .pl level should be working with abstractions of data, e.g. "a reserve", or "a biblio with attached items", not "a set of hashrefs containing a biblio, a biblioitem, and the associated item records." This latter is what Result/ResultSet represent.
Only if we don't take advantage of what DBIC is capable of. I agree, in the way we are using it now you are correct. That's why I've proposed what I have.
In the ES code, I've been keeping this in mind. For example, I can go (from memory):
my $it = Koha::Biblio->get_iterator; while (my $bib = $it->next) { add_to_index($bib); }
which means I don't have to care how they're stored. It also meant that it took very little work to refactor it to handle authorities, as everything had the same style of API. ResultSet stuff happened in the modules, but none of it was exposed to the script that was using those modules.
I don't see how this couldn't have been accomplished just as easily with DBIC. Biblios and authorities would also share the same DBIC methods.
Exactly. If you look at my accounts rewrite you'll see I didn't try to shoehorn everything into Result and ResultSet. I used a Koha module that uses Results and ResultSets in a sane fashion.
And that's fine. Just not at the .pl level. The job of modules is to put a sensible abstraction between the storage layer and the user interface layer. As soon as UI related code can touch the storage layer, you're violating that principle and causing maintenance headaches for the next person who comes along.
The UI isn't touching the storage layer, its touching an abstraction layer. I'm just not for the idea of abstracting our abstraction layer. I think we are in general agreement. I just disagree about hard and fast rules that could limit the real potential we have with DBIC. Kyle
Its worth keeping in mind that DBIC is an Object Relational mapper and that as such returns objects which are often better abstrations than those currently embodied in Koha's module approach which tend to fiddle with the representation of the entity not always very consistently. In a multitude of cases a ResultSet of x objects and their inherent getters and setters is all we need to construct the operations we need. In such cases wrapping the layer in another one for purely formal reasons contributes little except for more space to harbour bugs. DBIC allows you to add extra methods to these objects and I'm sure in time we'll identify plenty of these but we should be very thorough in reviewing whether we need them, how we've implemented them and whether the need is merely pointing up a shortcoming in the schema. Colin -- Colin Campbell Chief Software Engineer, PTFS Europe Limited Content Management and Library Solutions +44 (0) 800 756 6803 (phone) +44 (0) 7759 633626 (mobile) colin.campbell@ptfs-europe.com skype: colin_campbell2 http://www.ptfs-europe.com
I completely agree. You've described the issue more succinctly and eloquently than I have by far. Kyle On Thu, Sep 11, 2014 at 4:15 AM, Colin Campbell < colin.campbell@ptfs-europe.com> wrote:
Its worth keeping in mind that DBIC is an Object Relational mapper and that as such returns objects which are often better abstrations than those currently embodied in Koha's module approach which tend to fiddle with the representation of the entity not always very consistently. In a multitude of cases a ResultSet of x objects and their inherent getters and setters is all we need to construct the operations we need. In such cases wrapping the layer in another one for purely formal reasons contributes little except for more space to harbour bugs. DBIC allows you to add extra methods to these objects and I'm sure in time we'll identify plenty of these but we should be very thorough in reviewing whether we need them, how we've implemented them and whether the need is merely pointing up a shortcoming in the schema.
Colin
-- Colin Campbell Chief Software Engineer, PTFS Europe Limited Content Management and Library Solutions +44 (0) 800 756 6803 (phone) +44 (0) 7759 633626 (mobile) colin.campbell@ptfs-europe.com skype: colin_campbell2
http://www.ptfs-europe.com _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-----Original Message----- From: koha-devel-bounces@lists.koha-community.org [mailto:koha-devel- bounces@lists.koha-community.org] On Behalf Of Colin Campbell Sent: Thursday, 11 September 2014 6:15 PM To: koha-devel@lists.koha-community.org Subject: Re: [Koha-devel] Koha and DBIC
Its worth keeping in mind that DBIC is an Object Relational mapper and
Hi Colin: Well said! I think that the shortcomings in the schema are definitely part of the issue here. For instance, the "biblio" table isn't really an accurate representation of a "Biblio" object, even/especially via DBIC. That's because a proper "Biblio" object is actually an amalgam of "biblio" and "biblioitems". Likewise, a proper "Biblio" object should also be shared with "deletedbiblio" and "deletedbiblioitems". There's also "biblioimages". I think that's what Robin was referring to when he talked about the need for having Koha:: objects. As Robin was saying, DBIC is an abstraction layer for the database. It is a much more convenient way of dealing with the database, but it seems to me that it's still all about CRUD. As Jonathan was saying, the purpose of having Koha:: objects is to add logical functionality. To make it a "Koha" object. That said, I think Colin and Kyle have good points about not wanting to abstract the abstraction, as it does add overhead. DBIC can be slow. Plus, the more layers, the more possible bugs. But unless I'm missing something... DBIC can't give us a single "Biblio" object that would contain data from "biblio" and "biblioitems" or "deletedbiblio" and "deletedbiblioitems". We would need a "Koha" object. Now... that's probably because of shortcomings with the schema, like Colin mentioned. But... it seems to me that we're stuck with the schema for the moment. Maybe "Biblio" isn't a good example, as it's a difficult one. Maybe something simpler... "reserves" and "old_reserves" wouldn't work. "issues" and "old_issues" wouldn't work. Unless maybe I'm missing how you create the ResultSet classes. That could be. (The "biblio" and "biblioitems" example would still probably be a problem though?) I don't know how this DBIC idea could work in many cases. Take subscriptions. There are several tables that deal with subscriptions. Or "messages". Wouldn't we want a "Koha" class that interacts with multiple tables via DBIC? Objects are cool, because you can store data and methods in them, but it seems to me that we should be thinking about our objects as "things" rather than tables. A "Biblio" touches several tables, or could be populated by several different tables. There are 4 "oai_set*" tables but we'd only want 1 "oai_set" object. Maybe I'm thinking about this too simplistically or too abstractly, as I don't have a code example at hand. I keep going back to this "Biblio" idea though (perhaps because biblio records are so key to Koha). It would have something like "save" and "delete" methods. The save method would add/update the "biblio" and "biblioitems" tables. The delete method would delete from "biblio" and "biblioitems" and write to "deletedbiblio" and "deletedbiblio" items. Likewise, there could be "image" methods for controlling the images associated with that bib. I suppose the downside of the image methods would be... you wouldn't necessarily want to load the whole biblio object just to get to the images, but I suppose you wouldn't have to. Koha::Biblio could be really lightweight. You could just give it a biblionumber, then do all sorts of functions related to that biblionumber without loading the marcxml or the biblio/biblioitems data. I'm looking at http://sourceforge.net/p/bizwerks/bizwerks/ci/master/tree/lib/BizWerks/Schem a/ResultSet/Department.pm but I don't see how the children() method works. It would be nice to have something like that with Biblio as well, so you can get the "children items". The attached orders. The attached reservations. All which would be their own "Koha" objects. I don't quite understand how the custom ResultSet methods would work. Maybe you could have custom ResultSet methods for Biblio that would get attached orders and attached reservations which might be DBIC objects. Yet... we also have "import_items". So I'd rather have a "Koha" object for "Item". But then if we're using "Koha" objects for "Item" and "Biblio", why would we not have "Koha" objects for "Reserves" and "Orders" which are more than just data. They have logic. Can this item be placed on reserve? I think that's similar to the "apply_security_policy" example given at http://search.cpan.org/~ribasushi/DBIx-Class-0.08270/lib/DBIx/Class/ResultSe t.pm. This seems to imply an application-level object using the DBIC object. Maybe it is silly if we just build simple wrappers around simple DBIC objects, but I think a fair bit of the code in Koha is more complex than just CRUD. I'm skeptical but I'm intrigued. David Cook Systems Librarian Prosentient Systems 72/330 Wattle St, Ultimo, NSW 2007 that
as such returns objects which are often better abstrations than those currently embodied in Koha's module approach which tend to fiddle with the representation of the entity not always very consistently. In a multitude of cases a ResultSet of x objects and their inherent getters and setters is all we need to construct the operations we need. In such cases wrapping the layer in another one for purely formal reasons contributes little except for more space to harbour bugs. DBIC allows you to add extra methods to these objects and I'm sure in time we'll identify plenty of these but we should be very thorough in reviewing whether we need them, how we've implemented them and whether the need is merely pointing up a shortcoming in the schema.
Colin
-- Colin Campbell Chief Software Engineer, PTFS Europe Limited Content Management and Library Solutions +44 (0) 800 756 6803 (phone) +44 (0) 7759 633626 (mobile) colin.campbell@ptfs-europe.com skype: colin_campbell2
http://www.ptfs-europe.com _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha- community.org/ bugs : http://bugs.koha-community.org/
2014-09-12 3:31 GMT+02:00 David Cook <dcook@prosentient.com.au>:
That said, I think Colin and Kyle have good points about not wanting to abstract the abstraction, as it does add overhead. DBIC can be slow. Plus, the more layers, the more possible bugs.
We need to create a Koha API, a good and better API. For that the only way to do is to add Koha classes to wrap the DBIC ResultSet classes (note that we could extend it). I really don't think performance will be impacted here. The difference can be ignore. I disagree with the "more layers, more bugs" idea. An abstraction layer adds flexibility and a better overview. For instance, I think we should have a Koha::Record class. Biblio and Authority would be a Koha::Record. Do you think this new class adds more complexity? To me it adds consistence and the common code will be refactored in this class.
But unless I'm missing something... DBIC can't give us a single "Biblio" object that would contain data from "biblio" and "biblioitems" or "deletedbiblio" and "deletedbiblioitems". We would need a "Koha" object. Now... that's probably because of shortcomings with the schema, like Colin mentioned. But... it seems to me that we're stuck with the schema for the moment. Maybe "Biblio" isn't a good example, as it's a difficult one. Maybe something simpler...
Koha has a lot of complex concepts. Because it offers a lot of modules and possibilities to the users. Koha has to work for every configuration across the world. Biblio won't be a special case. So there are not reason to cut the class in 2 groups: the simple and the complex. All lot will need their own modules, because we need to be flexible and logical.
I'm skeptical but I'm intrigued.
Thanks to share your interest :) You managed to tell what I didn't.
On Fri, Sep 12, 2014 at 3:43 AM, Jonathan Druart < jonathan.druart@biblibre.com> wrote:
2014-09-12 3:31 GMT+02:00 David Cook <dcook@prosentient.com.au>:
That said, I think Colin and Kyle have good points about not wanting to abstract the abstraction, as it does add overhead. DBIC can be slow. Plus, the more layers, the more possible bugs.
We need to create a Koha API, a good and better API. For that the only way to do is to add Koha classes to wrap the DBIC ResultSet classes (note that we could extend it).
This ^. That's the solution. If we extend the resultset class then we can retain all the DBIC methods and retain the ability to leverage all the strengths of DBIC. For example, let's say we want to carp/die if a particular key isn't passed to create(), or maybe we any delete to fail if there's still a row in table X that's related to it. All we need to do is use inheritance so we can test our conditions, then call the parent's create(), or delete() or whatever method.
I really don't think performance will be impacted here. The difference can be ignore. I disagree with the "more layers, more bugs" idea. An abstraction layer adds flexibility and a better overview. For instance, I think we should have a Koha::Record class. Biblio and Authority would be a Koha::Record.
We can use multiple inheritance or Roles ( interfaces in OO terminology ) to achieve this. That way each Biblio and Authority share a common set of methods and can be used interchangeably in certain circumstances.
Do you think this new class adds more complexity? To me it adds consistence and the common code will be refactored in this class.
But unless I'm missing something... DBIC can't give us a single "Biblio"
object that would contain data from "biblio" and "biblioitems" or "deletedbiblio" and "deletedbiblioitems". We would need a "Koha" object. Now... that's probably because of shortcomings with the schema, like Colin mentioned. But... it seems to me that we're stuck with the schema for the moment. Maybe "Biblio" isn't a good example, as it's a difficult one. Maybe something simpler...
DBIC can't give us a single record object technically, because of our schema ( the tables should be merged into one at this point in time ). However, we can solve this in two ways. We can either just write $biblio->biblioitem->marcxml and say it's not a big deal, or we can just add methods for each of the biblioitem methods so we can just treat them as one object: sub marcxml { my $self = (@_); return $self->biblioitem->marcxml; } We can have a method that returns a MARC::Record, or perhaps we could even have our Result::Biblio inherit from MARC::Record, and our ResultSet::Biblio inherit from MARC::Batch. Koha has a lot of complex concepts. Because it offers a lot of modules
and possibilities to the users. Koha has to work for every configuration across the world. Biblio won't be a special case. So there are not reason to cut the class in 2 groups: the simple and the complex. All lot will need their own modules, because we need to be flexible and logical.
I absolutely agree. I think at the simplest we can use our Result and ResultSet classes, at the next level we can extend our Result and ResultSet classes, and at the far end of the spectrum we have fancy classes that don't map easily to the database but use DBIC objects to get things done.
I'm skeptical but I'm intrigued.
Thanks to share your interest :) You managed to tell what I didn't.
Agreed! Kyle
This appears to be an excellent read on ways to extend and enhance Result and ResultSet: https://github.com/castaway/dbix-class-book/blob/master/chapters/06-Componen... Kyle
I have written a bit of proof of concept code as an alternative implementation of http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10363 Please let me know what you think! If you compare the original proposed Koha::AuthorisedValue with my alternate implementation, I think you'll see how much less code is needed. This makes it easier to understand, lowers the barrier to entry for new developers, reduces the chance of bugs and reduces overhead. Kyle http://www.kylehall.info ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com )
2014-09-12 13:47 GMT+02:00 Kyle Hall <kyle.m.hall@gmail.com>:
Koha has a lot of complex concepts. Because it offers a lot of modules and possibilities to the users. Koha has to work for every configuration across the world. Biblio won't be a special case. So there are not reason to cut the class in 2 groups: the simple and the complex. All lot will need their own modules, because we need to be flexible and logical.
I absolutely agree. I think at the simplest we can use our Result and ResultSet classes, at the next level we can extend our Result and ResultSet classes, and at the far end of the spectrum we have fancy classes that don't map easily to the database but use DBIC objects to get things done.
But we cannot have 2 ways to instantiate an object. I think we should define how we would like to write code using the "new" API. For instance: # Insert a biblio my $biblio = Koha::Biblio->new($biblio_info)->insert (or create) # Delete a biblio my $biblio_2 = Koha::Biblio->fetch($biblionumber)->delete Does everybody agree we want to write something as easy as it? If yes, this should work for all objects. Even if the class if really short (and just tell "this class extends this one"), it should exist. I don't think they should be another way for "simple classes".
First question to be asked: why are we adopting an ORM? If we are using it to abstract the DB engine selection (i.e. making Koha usable with Postgres instead of only MySQL) I'm sure we are wasting our time. What's the point of rewriting the whole thing to just move away from an enterprise grade DB engine to another? Is it worth the trouble? I'm glad that using an ORM also serves that purpose, but... I think our project is complex enough to benefit from strictly sticking to design patterns, such as the repository pattern; in which the ORM takes care of persistence and nothing more, and we build our business logic on top of that. On that subject, I fully agree with Robin, that we shouldn't rely on hooking the ORM to solve any business logic issue: we need to have our own abstraction layer with a proper API we maintain and unit test (mocked tests, and let integration tests deal with the ORM interaction). Hooking the ORM mixes layers, businesses and problems. That's why it is so hard to write some tests for Koha. Also, our ERD needs some tweaking, to aid maintenance, code simplicity and to support an OO design [1]. For instance, a biblio should definitely have its table, and if biblioitems breaks the design, then we should just merge those tables. I wouldn't implement any hack on the code to preserve that discussed structure. I'd rather spend the time fixing it. So at that level, we should implement the repository pattern, with the following goals: - Consistent API, independent of the DB model (people use our API, no need to know how we built our DB structure). - Separation of concerns between layers (easier to code, easier to test, etc) - Let the ORM take care of DB stuff and use the spare time to talk about cats and food. [2] I'd also like to have a RESTful layer for that API, that should be straightforward to maintain: endpoint routing, authorization layer, call to the right method, output. It is outside the scope of this thread, but a better separation of concerns might make it easier to achieve in a short term. We could the gradually move away from our .pl mess, that contains lots of business logic that doesn't get tested. Best regards Tomás [1] We've just approved a new coding guideline addition in that sense. [2] I forgot to mention beer. On Wed, Sep 10, 2014 at 10:19 PM, Robin Sheat <robin@catalyst.net.nz> wrote:
Kyle Hall schreef op wo 10-09-2014 om 20:51 [-0400]:
It's funny, I've been trying to think of an example search to justify this and I can't really think of one where we can't use existing Result relationships to get our data. Maybe find() and single() are really all we need from pl files, but I don't think a hard and fast rule against search() is a good idea.
We'd still be coupling to the data storage schema, which is bad. It's exactly the same logic that says not to have SQL in the .pl files.
Result and ResultSet are part of the Koha namespace. I also feel that
Only in the sense that they have Koha:: in the name. They are an API to the database, and are generated directly from the database schema. They are not an API that is designed to provide logical access to whatever it is.
you aren't quite correct about how one modules need to know the change a present the same interface. I can't count the number of times I've added a new parameter to an existing subroutine and had to update every function call in many perl files. Now that hashref's are more common, it's become less of a problem but it's still a problem that exists.
"It's been broken in the past" isn't a justification for "so we should keep it broken." As we go to a more OO and thought out method of writing things (again, especially with the Koha:: stuff), we are moving away from "here's a hashref with the fields from the biblio table" to "here's an object that represents a biblio, with accessors and modifiers." We want more of this, and less things going and talking to the database themselves.
I just don't think this is a entirely valid point. Remember, Result and ResultSet methods are no different than methods anywhere else in Koha and C4.
They are totally different. They are much closer to an abstraction of the database, compared to an abstraction of whatever it is the data is representing. And the code at the .pl level should be working with abstractions of data, e.g. "a reserve", or "a biblio with attached items", not "a set of hashrefs containing a biblio, a biblioitem, and the associated item records." This latter is what Result/ResultSet represent.
In the ES code, I've been keeping this in mind. For example, I can go (from memory):
my $it = Koha::Biblio->get_iterator; while (my $bib = $it->next) { add_to_index($bib); }
which means I don't have to care how they're stored. It also meant that it took very little work to refactor it to handle authorities, as everything had the same style of API. ResultSet stuff happened in the modules, but none of it was exposed to the script that was using those modules.
Exactly. If you look at my accounts rewrite you'll see I didn't try to shoehorn everything into Result and ResultSet. I used a Koha module that uses Results and ResultSets in a sane fashion.
And that's fine. Just not at the .pl level. The job of modules is to put a sensible abstraction between the storage layer and the user interface layer. As soon as UI related code can touch the storage layer, you're violating that principle and causing maintenance headaches for the next person who comes along.
-- Robin Sheat Catalyst IT Ltd. ✆ +64 4 803 2204 GPG: 5FA7 4B49 1E4D CAA4 4C38 8505 77F5 B724 F871 3BDF
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Tomás Cohen Arazi Prosecretaría de Informática Universidad Nacional de Córdoba ✆ +54 351 5353750 ext 13168 GPG: B76C 6E7C 2D80 551A C765 E225 0A27 2EA1 B2F3 C15F
First question to be asked: why are we adopting an ORM?
If we are using it to abstract the DB engine selection (i.e. making Koha usable with Postgres instead of only MySQL) I'm sure we are wasting our time. What's the point of rewriting the whole thing to just move away from an enterprise grade DB engine to another? Is it worth the trouble? I'm glad that using an ORM also serves that purpose, but...
In the past if not now, there has been concerns about the future of MySQL. I think there has been a strong interest in enabling Koha to use Postgres at the very least.
I think our project is complex enough to benefit from strictly sticking to design patterns, such as the repository pattern; in which the ORM takes care of persistence and nothing more, and we build our business logic on top of that. On that subject, I fully agree with Robin, that we shouldn't rely on hooking the ORM to solve any business logic issue: we need to have our own abstraction layer with a proper API we maintain and unit test (mocked tests, and let integration tests deal with the ORM interaction). Hooking the ORM mixes layers, businesses and problems. That's why it is so hard to write some tests for Koha.
I can totally agree with this, except I don't see where we are using any design patterns with Koha. I we need to retain the pattern of Objects and Object Sets. But we already have those though DBIC! In the end we could easily yank out DBIC and replace it with something else so long as it provides the same methods ( create, update, delete, find, search, etc ). I am troubled by the idea that we should wrap all our dbic classes in yet more classes. Every example I've seen of this has more code by a factor of almost 10. I don't know if Koha is so complex that it requires a repository pattern. Look at this example: http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31607&action=diff#a/Koha/Schema/ResultSet/Reserve.pm_sec1 How much more difficult will this be for developers, and how much more overhead will it require if we wrap our objects in more objects? We'd have to fetch the Row objects, wrap them in KohaRow objects, wrap those in a KohaRowSet, and return them. Certainly, but far more complicated. That being said, if the majority agree, than I will be the first to jump aboard. However, there have definitely been others who are reticent as well.
Also, our ERD needs some tweaking, to aid maintenance, code simplicity and to support an OO design [1]. For instance, a biblio should definitely have its table, and if biblioitems breaks the design, then we should just merge those tables. I wouldn't implement any hack on the code to preserve that discussed structure. I'd rather spend the time fixing it.
I totally agree. The biblio and biblioitems tables need to be merged. Finding a volunteer will be the hard part!
So at that level, we should implement the repository pattern, with the following goals: - Consistent API, independent of the DB model (people use our API, no need to know how we built our DB structure).
Is this so other software can make use of Koha's functions and methods?
- Separation of concerns between layers (easier to code, easier to test, etc)
I will concede that unit testing would be much easier if we could instantiate objects and object sets without using the database. It *is* trivial to create Result objects without touching the db, but ResultSets are another matter. However, it seems like it may be possible: http://modernperlbooks.com/mt/2011/05/testing-dbix-models-without-the-databa...
- Let the ORM take care of DB stuff and use the spare time to talk about cats and food. [2]
Don't forget about dogs! We've got plenty of dog lovers in out community ; )
I'd also like to have a RESTful layer for that API, that should be straightforward to maintain: endpoint routing, authorization layer, call to the right method, output. It is outside the scope of this thread, but a better separation of concerns might make it easier to achieve in a short term. We could the gradually move away from our .pl mess, that contains lots of business logic that doesn't get tested.
This I completely agree with. I think all outside systems should interact with Koha though a well designed restful interface. I'm hoping to get started on this soon. Jesse Weaver has already written some great code I can use as a start. Kyle
I am troubled by the idea that we should wrap all our dbic classes in yet more classes. Every example I've seen of this has more code by a factor of almost 10. I don't know if Koha is so complex that it requires a repository pattern. I think that it would require rewriting Koha. This changeover might just be too complex for us.
How much more difficult will this be for developers, and how much more overhead will it require if we wrap our objects in more objects? We'd have to fetch the Row objects, wrap them in KohaRow objects, wrap those in a KohaRowSet, and return them. Certainly, but far more complicated. I would say: Leave all storage related actions in Koha::Schema. KohaRow does not make sense to me. Furthermore, define the objects that actually have 'real' business logic and put that in some Koha::Object.
Marcel
Yes. I'm imagining something along the lines of Koha::Object, and Koha::Object::Set which would have all the boilerplate we need for general use ( get, set, find, search, etc ). Then all our table-tied objects would inherit from Koha::Object and a set of those objects would inherit from Koha::Object::Set. Both of those classes can be DBIC-aware internally. Internally Koha::Object would have a DBIC Result as a property, which can be used independently from the database ( which would be good for unit testing ). Each Koha::Object::Set would normally keep only a ResultSet internally and work on that until asked to return a Koha::Object, at which point it would wrap each Result in a Koha::Object and store them internally, or return an array of them depending on what type of return value the method was called with. I would be willing to write up Koha::Object and Koha::Object::Set if this is what Robin, Tomas et. al. are looking for. In this way we have DBIC totally encapsulated so the code using Koha::Object and Koha::Object::Set's is totally DBIC unaware but internally is DBIC aware. In addition, if we have these two classes to inherit from, it will reduce the amount of code we must write, and make it much easier for new developers. One we have Object and Object::Set written, we don't have to rewrite our CRUD boilerplate for each and every new class we add. What do you think? Kyle http://www.kylehall.info ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com ) On Mon, Sep 15, 2014 at 3:11 PM, Marcel de Rooy <M.de.Rooy@rijksmuseum.nl> wrote:
I am troubled by the idea that we should wrap all our dbic classes in yet more classes. Every example I've seen of this has more code by a factor of almost 10. I don't know if Koha is so complex that it requires a repository pattern. I think that it would require rewriting Koha. This changeover might just be too complex for us.
How much more difficult will this be for developers, and how much more overhead will it require if we wrap our objects in more objects? We'd have to fetch the Row objects, wrap them in KohaRow objects, wrap those in a KohaRowSet, and return them. Certainly, but far more complicated. I would say: Leave all storage related actions in Koha::Schema. KohaRow does not make sense to me. Furthermore, define the objects that actually have 'real' business logic and put that in some Koha::Object.
Marcel
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
\o/ If I understood correctly, it's exactly what I had in mind, so I totally agree! 2014-09-16 13:16 GMT+02:00 Kyle Hall <kyle.m.hall@gmail.com>:
Yes. I'm imagining something along the lines of Koha::Object, and Koha::Object::Set which would have all the boilerplate we need for general use ( get, set, find, search, etc ). Then all our table-tied objects would inherit from Koha::Object and a set of those objects would inherit from Koha::Object::Set. Both of those classes can be DBIC-aware internally.
Internally Koha::Object would have a DBIC Result as a property, which can be used independently from the database ( which would be good for unit testing ). Each Koha::Object::Set would normally keep only a ResultSet internally and work on that until asked to return a Koha::Object, at which point it would wrap each Result in a Koha::Object and store them internally, or return an array of them depending on what type of return value the method was called with.
I would be willing to write up Koha::Object and Koha::Object::Set if this is what Robin, Tomas et. al. are looking for.
In this way we have DBIC totally encapsulated so the code using Koha::Object and Koha::Object::Set's is totally DBIC unaware but internally is DBIC aware.
In addition, if we have these two classes to inherit from, it will reduce the amount of code we must write, and make it much easier for new developers. One we have Object and Object::Set written, we don't have to rewrite our CRUD boilerplate for each and every new class we add.
What do you think?
Kyle
http://www.kylehall.info ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com )
On Mon, Sep 15, 2014 at 3:11 PM, Marcel de Rooy <M.de.Rooy@rijksmuseum.nl> wrote:
I am troubled by the idea that we should wrap all our dbic classes in yet more classes. Every example I've seen of this has more code by a factor of almost 10. I don't know if Koha is so complex that it requires a repository pattern. I think that it would require rewriting Koha. This changeover might just be too complex for us.
How much more difficult will this be for developers, and how much more overhead will it require if we wrap our objects in more objects? We'd have to fetch the Row objects, wrap them in KohaRow objects, wrap those in a KohaRowSet, and return them. Certainly, but far more complicated. I would say: Leave all storage related actions in Koha::Schema. KohaRow does not make sense to me. Furthermore, define the objects that actually have 'real' business logic and put that in some Koha::Object.
Marcel
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
* Jonathan Druart (jonathan.druart@biblibre.com) wrote:
\o/ If I understood correctly, it's exactly what I had in mind, so I totally agree!
2014-09-16 13:16 GMT+02:00 Kyle Hall <kyle.m.hall@gmail.com>:
Yes. I'm imagining something along the lines of Koha::Object, and Koha::Object::Set which would have all the boilerplate we need for general use ( get, set, find, search, etc ). Then all our table-tied objects would inherit from Koha::Object and a set of those objects would inherit from Koha::Object::Set. Both of those classes can be DBIC-aware internally.
Internally Koha::Object would have a DBIC Result as a property, which can be used independently from the database ( which would be good for unit testing ). Each Koha::Object::Set would normally keep only a ResultSet internally and work on that until asked to return a Koha::Object, at which point it would wrap each Result in a Koha::Object and store them internally, or return an array of them depending on what type of return value the method was called with.
I would be willing to write up Koha::Object and Koha::Object::Set if this is what Robin, Tomas et. al. are looking for.
In this way we have DBIC totally encapsulated so the code using Koha::Object and Koha::Object::Set's is totally DBIC unaware but internally is DBIC aware.
In addition, if we have these two classes to inherit from, it will reduce the amount of code we must write, and make it much easier for new developers. One we have Object and Object::Set written, we don't have to rewrite our CRUD boilerplate for each and every new class we add.
What do you think?
Kyle
I cant answer for Tomas or Robin, but this is not what I was thinking at all. We don't need more boilerplate setters and getters, DBIX::Class does this for us. What we don't want, is DBIx::Class or for that matter Koha::Object calls in the .pl files I dont think we need to build an abstraction over DBIx::Class, certainly thats not what Robin was asking. What we don't want to see is DBIx::Class calls in the .pl files they should live in modules (objects) in the Koha:: namespace. In fact all of our pl files that are longer than about 40 lines should all be rewritten, all the business logic should be in the modules. I think there may be a misunderstanding, we dont need to abstract database access over top of DBIx::Class .. thats what it's for. What we dont want to be doing is setting and getting from inside the pl files. I think if you look at the Koha::Biblio modules etc that Robin has written they illustrate this. Hopefully this is clear, to restate -1 for DBIx::Class calls in the .pl -1 for Koha::Object and Koha::Object::Set +1 for proper objects like Koha::Biblio, Koha::Account, Koha::Serial etc Chris
http://www.kylehall.info ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com )
On Mon, Sep 15, 2014 at 3:11 PM, Marcel de Rooy <M.de.Rooy@rijksmuseum.nl> wrote:
I am troubled by the idea that we should wrap all our dbic classes in yet more classes. Every example I've seen of this has more code by a factor of almost 10. I don't know if Koha is so complex that it requires a repository pattern. I think that it would require rewriting Koha. This changeover might just be too complex for us.
How much more difficult will this be for developers, and how much more overhead will it require if we wrap our objects in more objects? We'd have to fetch the Row objects, wrap them in KohaRow objects, wrap those in a KohaRowSet, and return them. Certainly, but far more complicated. I would say: Leave all storage related actions in Koha::Schema. KohaRow does not make sense to me. Furthermore, define the objects that actually have 'real' business logic and put that in some Koha::Object.
Marcel
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Chris Cormack Catalyst IT Ltd. +64 4 803 2238 PO Box 11-053, Manners St, Wellington 6142, New Zealand
Chris, I tent to agree with you, but a Koha::Object makes sense to me. For instance, bug 12891 [1] shows us that we need an object to refactor common stuffs. [1] http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=31515 2014-09-17 9:50 GMT+02:00 Chris Cormack <chrisc@catalyst.net.nz>:
* Jonathan Druart (jonathan.druart@biblibre.com) wrote:
\o/ If I understood correctly, it's exactly what I had in mind, so I totally agree!
2014-09-16 13:16 GMT+02:00 Kyle Hall <kyle.m.hall@gmail.com>:
Yes. I'm imagining something along the lines of Koha::Object, and Koha::Object::Set which would have all the boilerplate we need for general use ( get, set, find, search, etc ). Then all our table-tied objects would inherit from Koha::Object and a set of those objects would inherit from Koha::Object::Set. Both of those classes can be DBIC-aware internally.
Internally Koha::Object would have a DBIC Result as a property, which can be used independently from the database ( which would be good for unit testing ). Each Koha::Object::Set would normally keep only a ResultSet internally and work on that until asked to return a Koha::Object, at which point it would wrap each Result in a Koha::Object and store them internally, or return an array of them depending on what type of return value the method was called with.
I would be willing to write up Koha::Object and Koha::Object::Set if this is what Robin, Tomas et. al. are looking for.
In this way we have DBIC totally encapsulated so the code using Koha::Object and Koha::Object::Set's is totally DBIC unaware but internally is DBIC aware.
In addition, if we have these two classes to inherit from, it will reduce the amount of code we must write, and make it much easier for new developers. One we have Object and Object::Set written, we don't have to rewrite our CRUD boilerplate for each and every new class we add.
What do you think?
Kyle
I cant answer for Tomas or Robin, but this is not what I was thinking at all. We don't need more boilerplate setters and getters, DBIX::Class does this for us.
What we don't want, is DBIx::Class or for that matter Koha::Object calls in the .pl files
I dont think we need to build an abstraction over DBIx::Class, certainly thats not what Robin was asking. What we don't want to see is DBIx::Class calls in the .pl files they should live in modules (objects) in the Koha:: namespace.
In fact all of our pl files that are longer than about 40 lines should all be rewritten, all the business logic should be in the modules.
I think there may be a misunderstanding, we dont need to abstract database access over top of DBIx::Class .. thats what it's for. What we dont want to be doing is setting and getting from inside the pl files.
I think if you look at the Koha::Biblio modules etc that Robin has written they illustrate this.
Hopefully this is clear, to restate
-1 for DBIx::Class calls in the .pl -1 for Koha::Object and Koha::Object::Set +1 for proper objects like Koha::Biblio, Koha::Account, Koha::Serial etc
Chris
http://www.kylehall.info ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com )
On Mon, Sep 15, 2014 at 3:11 PM, Marcel de Rooy <M.de.Rooy@rijksmuseum.nl> wrote:
I am troubled by the idea that we should wrap all our dbic classes in yet more classes. Every example I've seen of this has more code by a factor of almost 10. I don't know if Koha is so complex that it requires a repository pattern. I think that it would require rewriting Koha. This changeover might just be too complex for us.
How much more difficult will this be for developers, and how much more overhead will it require if we wrap our objects in more objects? We'd have to fetch the Row objects, wrap them in KohaRow objects, wrap those in a KohaRowSet, and return them. Certainly, but far more complicated. I would say: Leave all storage related actions in Koha::Schema. KohaRow does not make sense to me. Furthermore, define the objects that actually have 'real' business logic and put that in some Koha::Object.
Marcel
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
_______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Chris Cormack Catalyst IT Ltd. +64 4 803 2238 PO Box 11-053, Manners St, Wellington 6142, New Zealand
A Koha::Object sounds like extremely poor design. What kind of object is a Koha::Object, what are its properties, when we say a Koha::Biblio isa Koha::Object what does that mean? In practice what meaningful properties it has it inherits from Koha::Schema::Result::Biblio which is liable to be a better representation than the vaguer Onject. In practice I've recently written some scripts which in first version used some abstractions built around dbic ResultSets, As I added to them I stripped out out the abstractions and used the dbic result sets as I found this generated cleaner, more straightforward code. Colin -- Colin Campbell Chief Software Engineer, PTFS Europe Limited Content Management and Library Solutions +44 (0) 800 756 6803 (phone) +44 (0) 7759 633626 (mobile) colin.campbell@ptfs-europe.com skype: colin_campbell2 http://www.ptfs-europe.com
I agree. I'd rather use DBIC directly and extend what we already have. It's just an idea if the conclusion is we can't do that. Kyle http://www.kylehall.info ByWater Solutions ( http://bywatersolutions.com ) Meadville Public Library ( http://www.meadvillelibrary.org ) Crawford County Federated Library System ( http://www.ccfls.org ) Mill Run Technology Solutions ( http://millruntech.com ) On Wed, Sep 17, 2014 at 6:49 AM, Colin Campbell < colin.campbell@ptfs-europe.com> wrote:
A Koha::Object sounds like extremely poor design. What kind of object is a Koha::Object, what are its properties, when we say a Koha::Biblio isa Koha::Object what does that mean? In practice what meaningful properties it has it inherits from Koha::Schema::Result::Biblio which is liable to be a better representation than the vaguer Onject.
In practice I've recently written some scripts which in first version used some abstractions built around dbic ResultSets, As I added to them I stripped out out the abstractions and used the dbic result sets as I found this generated cleaner, more straightforward code.
Colin
-- Colin Campbell Chief Software Engineer, PTFS Europe Limited Content Management and Library Solutions +44 (0) 800 756 6803 (phone) +44 (0) 7759 633626 (mobile) colin.campbell@ptfs-europe.com skype: colin_campbell2
http://www.ptfs-europe.com _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
Chris, I'm a bit lost here. The idea behind Koha::Object is that we have a common base class from which to derive Koha::Biblio, Koha::Serial and so forth. That is, this class would be used as the base for classes that have a simple 1 to 1 table mapping to encapsulate DBIC while maintaining a consistent set of methods across our many classes. ObjectSet would give us set iterators, set operations, and be a better place to have methods that return a set ( e.g. Koha::Issue::Set::GetOverdues would return a collection of overdue checkouts ). I'm not sure why you disagree with this idea. Could you expand a bit? Kyle
-1 for DBIx::Class calls in the .pl -1 for Koha::Object and Koha::Object::Set +1 for proper objects like Koha::Biblio, Koha::Account, Koha::Serial etc
Chris
I would say: Leave all storage related actions in Koha::Schema. KohaRow does not make sense to me. Furthermore, define the objects that actually have 'real' business logic and put that in some Koha::Object.
Just to add to the confusion ;) When I wrote "some Koha::Object" here, I was not literally suggesting Koha::Object, I was thinking of something like Koha::Biblio or Koha::Patron etc. I would not suggest to add such an object for every table in Koha. We should clearly define which objects(!) we really need, having sufficient business logic etc. to justify an interface to DBIx.
Greetings,
Just to add to the confusion ;) When I wrote "some Koha::Object" here, I was not literally suggesting Koha::Object, I was thinking of something like Koha::Biblio or Koha::Patron etc. That’s what I understood. So physically, we may have the tables we currently have accessed via DBIC, but the Koha::Biblio object would blend DBIC objects related to Biblios and Items. Right? This would mean that Koha::Biblio would provide whatever methods regardless of the backend structure. GPML, Mark Tompsett
On Wed, Sep 17, 2014 at 08:50:37AM -0400, Mark Tompsett wrote:
Greetings,
Just to add to the confusion ;) When I wrote "some Koha::Object" here, I was not literally suggesting Koha::Object, I was thinking of something like Koha::Biblio or Koha::Patron etc. That’s what I understood. So physically, we may have the tables we currently have accessed via DBIC, but the Koha::Biblio object would blend DBIC objects related to Biblios and Items. Right? This would mean that Koha::Biblio would provide whatever methods regardless of the backend structure.
Trouble is it sidesteps fixing the schema. The interplay between biblio and biblioitems will remain a problem for maintenance in future. There is already special code added to link the classes because their relationship is not maintained by the database. Special cases tend to grow over time and spawn more special cases. C. -- Colin Campbell Chief Software Engineer, PTFS Europe Limited Content Management and Library Solutions +44 (0) 800 756 6803 (phone) +44 (0) 7759 633626 (mobile) colin.campbell@ptfs-europe.com skype: colin_campbell2 http://www.ptfs-europe.com
On 18 September 2014 1:49:34 am NZST, Colin Campbell <colin.campbell@ptfs-europe.com> wrote:
Greetings,
Just to add to the confusion ;) When I wrote "some Koha::Object" here, I was not literally suggesting Koha::Object, I was thinking of something like Koha::Biblio or Koha::Patron etc. That’s what I understood. So physically, we may have the tables we currently have accessed via DBIC, but the Koha::Biblio object would blend DBIC objects related to Biblios and Items. Right? This would mean
On Wed, Sep 17, 2014 at 08:50:37AM -0400, Mark Tompsett wrote: that Koha::Biblio would provide whatever methods regardless of the backend structure.
Trouble is it sidesteps fixing the schema. The interplay between biblio and biblioitems will remain a problem for maintenance in future. There is already special code added to link the classes because their relationship is not maintained by the database. Special cases tend to grow over time and spawn more special cases.
It's not an either/or proposition, having Koha::Biblio doesn't mean you can't fix the schema too. It just means all the business logic for dealing with it is in that module/object. Instead of strewn about in scripts all over the place. Chris
C.
-- Colin Campbell Chief Software Engineer, PTFS Europe Limited Content Management and Library Solutions +44 (0) 800 756 6803 (phone) +44 (0) 7759 633626 (mobile) colin.campbell@ptfs-europe.com skype: colin_campbell2
http://www.ptfs-europe.com _______________________________________________ Koha-devel mailing list Koha-devel@lists.koha-community.org http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/
-- Sent from my Android device with K-9 Mail. Please excuse my brevity.
participants (11)
-
Chris Cormack -
Colin Campbell -
David Cook -
Jonathan Druart -
Kyle Hall -
Marcel de Rooy -
Mark Tompsett -
Philippe Blouin -
Robin Sheat -
Tomas Cohen Arazi -
Zeno Tajoli