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.

>