[Koha-devel] Coding patterns discussion

[Tomas Cohen Arazi]

El mié., 14 sept. 2016 a las 4:58, Jonathan Druart (<
jonathan.druart at bugs.koha-community.org>) escribió:

> Hi,
>
> We used to have problems with transactions, if they were used from the
> a subroutine/method which was called from tests (which use
> transactions as well), it did not work.
> Two transactions could not be started together.
>

I recall nested transactions were problematic.


> Now that we are using DBIx::Class transactions it works correctly and
> we will be able to use it in the codebase everywhere we need it.
> I try to use it as soon as it's useful (see
> Koha::Acquisition::Currency->store and bug 16907
> Koha::Patrons->delete).
>

Yes! The point of my email, was to highlight the possibility we now have,
and gather opinions on how to better do it. I wanted to raise this on a dev
meeting. We have a tradition of letting patches speak, but some healthy
discussion about this could let us reach some consensus, and guide new devs
into good coding practices.


> Regarding the try-catch module, the following is fine by me:
> use Koha::Exceptions;
> use Try::Tiny;
> sub boom {
>     Koha::Exceptions::Exception->throw;
> }
> try {
>     boom();
> } catch {
>     if ( ref($_) eq 'Koha::Exceptions::Exception' ) {
>         # Do that
>     } elsif ( ref($_) eq 'Koha::Exceptions::AnotherException' ) {
>         # do something else
>     }
> }
>

Yes, I like the Try::Tiny lib and would propose to just start using it.
Your code is simple to read, and a clear improvement! Koha::Virtualshelf
already raises exceptions and shelves.pl catches them on another way, in
master!
I mentioned TryCatch only because its syntax is sexier IMHO, but the main
point was, again, to highlight the possible use of some convenient
try/catch lib on the Koha:: namespace, which in conjuntcion with
transactions would make our code quite reliable and readable.

You should also have a look at Bug 13995 - Proper Exception handling
>

I will, I recall the bug got stuck because Olli introduced one file per
exception. It is handy for editor's autocomplete features, but it looked
awkward. That's why I propose splitting exceptions files on a per-package
basis.

Cheers
-- 
Tomás Cohen Arazi
Theke Solutions (https://theke.io <http://theke.io/>)
✆ +54 9351 3513384
GPG: B2F3C15F
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.koha-community.org/pipermail/koha-devel/attachments/20160914/7826075d/attachment-0001.html>