<div dir="ltr"><div><br class="">First question to be asked: why are we adopting an ORM?</div><div><br></div><div>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.</div><div>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...</div><div><br></div>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.<div><div><br></div><div>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.<br><div><br></div><div>So at that level, we should implement the repository pattern, with the following goals:</div><div>- Consistent API, independent of the DB model (people use our API, no need to know how we built our DB structure).</div><div>- Separation of concerns between layers (easier to code, easier to test, etc)</div><div>- Let the ORM take care of DB stuff and use the spare time to talk about cats and food. [2]</div><div><br></div></div><div>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.</div><div><br></div><div>Best regards</div><div>Tomás</div><div><br></div></div><div>[1] We've just approved a new coding guideline addition in that sense.</div><div>[2] I forgot to mention beer.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Sep 10, 2014 at 10:19 PM, Robin Sheat <span dir="ltr"><<a href="mailto:robin@catalyst.net.nz" target="_blank">robin@catalyst.net.nz</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Kyle Hall schreef op wo 10-09-2014 om 20:51 [-0400]:<br>
<span class=""><br>
<br>
> It's funny, I've been trying to think of an example search to justify<br>
> this and I can't really think of one where we can't use existing<br>
> Result relationships to get our data. Maybe find() and single() are<br>
> really all we need from pl files, but I don't think a hard and fast<br>
> rule against search() is a good idea.<br>
<br>
</span>We'd still be coupling to the data storage schema, which is bad. It's<br>
exactly the same logic that says not to have SQL in the .pl files.<br>
<span class=""><br>
<br>
> Result and ResultSet are part of the Koha namespace. I also feel that<br>
<br>
</span>Only in the sense that they have Koha:: in the name. They are an API to<br>
the database, and are generated directly from the database schema. They<br>
are not an API that is designed to provide logical access to whatever it<br>
is.<br>
<span class=""><br>
>  you aren't quite correct about how one modules need to know the<br>
> change a present the same interface. I can't count the number of times<br>
> I've added a new parameter to an existing subroutine and had to update<br>
> every function call in many perl files. Now that hashref's are more<br>
> common, it's become less of a problem but it's still a problem that<br>
> exists.<br>
<br>
</span>"It's been broken in the past" isn't a justification for "so we should<br>
keep it broken." As we go to a more OO and thought out method of writing<br>
things (again, especially with the Koha:: stuff), we are moving away<br>
from "here's a hashref with the fields from the biblio table" to "here's<br>
an object that represents a biblio, with accessors and modifiers." We<br>
want more of this, and less things going and talking to the database<br>
themselves.<br>
<span class=""><br>
> I just don't think this is a entirely valid point. Remember, Result<br>
> and ResultSet methods are no different than methods anywhere else in<br>
> Koha and C4.<br>
<br>
</span>They are totally different. They are much closer to an abstraction of<br>
the database, compared to an abstraction of whatever it is the data is<br>
representing. And the code at the .pl level should be working with<br>
abstractions of data, e.g. "a reserve", or "a biblio with attached<br>
items", not "a set of hashrefs containing a biblio, a biblioitem, and<br>
the associated item records." This latter is what Result/ResultSet<br>
represent.<br>
<br>
In the ES code, I've been keeping this in mind. For example, I can go<br>
(from memory):<br>
<br>
my $it = Koha::Biblio->get_iterator;<br>
while (my $bib = $it->next) {<br>
   add_to_index($bib);<br>
}<br>
<br>
which means I don't have to care how they're stored. It also meant that<br>
it took very little work to refactor it to handle authorities, as<br>
everything had the same style of API. ResultSet stuff happened in the<br>
modules, but none of it was exposed to the script that was using those<br>
modules.<br>
<span class=""><br>
<br>
> Exactly. If you look at my accounts rewrite you'll see I didn't try to<br>
> shoehorn everything into Result and ResultSet. I used a Koha module<br>
> that uses Results and ResultSets in a sane fashion.<br>
<br>
</span>And that's fine. Just not at the .pl level. The job of modules is to put<br>
a sensible abstraction between the storage layer and the user interface<br>
layer. As soon as UI related code can touch the storage layer, you're<br>
violating that principle and causing maintenance headaches for the next<br>
person who comes along.<br>
<div class="HOEnZb"><div class="h5">><br>
<br>
--<br>
Robin Sheat<br>
Catalyst IT Ltd.<br>
✆ +64 4 803 2204<br>
GPG: 5FA7 4B49 1E4D CAA4 4C38  8505 77F5 B724 F871 3BDF<br>
</div></div><br>_______________________________________________<br>
Koha-devel mailing list<br>
<a href="mailto:Koha-devel@lists.koha-community.org">Koha-devel@lists.koha-community.org</a><br>
<a href="http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel" target="_blank">http://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel</a><br>
website : <a href="http://www.koha-community.org/" target="_blank">http://www.koha-community.org/</a><br>
git : <a href="http://git.koha-community.org/" target="_blank">http://git.koha-community.org/</a><br>
bugs : <a href="http://bugs.koha-community.org/" target="_blank">http://bugs.koha-community.org/</a><br></blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr"><div>Tomás Cohen Arazi</div><div>Prosecretaría de Informática</div><div>Universidad Nacional de Córdoba</div><div>✆ +54 351 5353750 ext 13168</div><div>GPG: B76C 6E7C 2D80 551A C765  E225 0A27 2EA1 B2F3 C15F</div></div>
</div>