The problem is that the Modules containing the external calls are large and rambling. They export into our namespace lots we dont need. Ok... But it is what we have at the moment. And we cannot rewrite all Koha at once... :D Maybe we could try and use more EXPORT_OK and then create tags for actions (circulation, etc..) and try and select the tags we want when we "USE" a module. This could be a step, but would not be enough reading
Le 17/12/2010 18:00, Colin Campbell a écrit : the rest of your message. So ... Let's try and see more.
It is very hard when calling those routines to know confidently what side effects they may have.
So we get a checkout( borrowercard, itemcard) { itemstuff = GetItem(itemcard) blah ... blah... ItemXYZ(itemstuff->itemid); blah .. blah ... ItemABC($var)
etc; and we've got GetItem ItemXYZ ItemABC and loads more Item routines imported. And we probably duplicate external accesses across them.
But if we have a wrapper class around item and do checkout( borrowercard, itemcard) { my $item = Item->new(ittemcard); blah ... blah ... $item->zyx(); blah .. blah $item->abc()
mmmm... I think I get you. Would need even deeper refactoring then... Could we not get step by step ? Or maybe there is some background or easy changes I miss.
item is lexical, if necessary it can call its destructor when it goes out of scope. We don't have to refetch data for different calls as it persists while the variable persists and we've not imported a pile of package variables into our namespace. Also in our business logic we can avoid those GetItem then a long if else checking varying things and replace that with a method call.
For the kind of cases you are mentioning we have simple boolean methods: $item->renewable();
Well, still renewable would have to check reserves.... and therefore would use a reserve object in items... (And an item is not renewable per se... It is an issue which can or cannot (impossible) or maynot (you need confirmation) be renewed depending on circumstances. But I got your idea) I wanted to break circular dependencies between Reserves and Circulation and Reserves or Reserves and Items (see the picture brought along). Maybe it is only with object definition and paradygm that we could solve this out. But what occurs to me was that it might have been quite easy to split true atomic object methods (Add, Edit, delete, search) and the definition of ability to do a function (would it be Decision Table driven) that might take into account the whole environment of an object... For instance : Say I want to Delete a Biblio. Is it linked to a subscription, to an order, to an issue ? Then donot delete that... Or delete all the related objects... Or ask confirmation... I think it is good that it is a method. But it might be misplaced in C4::Biblio. Since makes use of the global environment. And therefore makes intensive usage of use C4:: ... This is why I wanted to confine those types of functions into a kind of "Controller" object... This was what I realized with the picture and code analysis... This was my reflection, and what I proposed to work on and have feedback on. I appreciate your feedback, and hope though that with Data Persistence, we will be able to get Koha up to better and widely used coding practises and standards of PERL (using Moose, and DBIx::Class or Schema, maybe KiokuDB and Modern::Perl), and really ease the QA job eventually. Since Data Persistence is one of the battles for 3.4 http://wiki.koha-community.org/wiki/RFCs_for_Koha_3.4 , let's join forces in that battle, win that one, all together... Template::Toolkit is really nicely coped by Catalyst. Hopes that helps. -- Henri-Damien LAURENT
