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. 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). 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 } } Not very different from the TryCatch syntax, we just have 1 more level of indent. You should also have a look at Bug 13995 - Proper Exception handling Cheers, Jonathan 2016-09-12 19:32 GMT+01:00 Tomas Cohen Arazi <tomascohen@gmail.com>:
I've been looking at our coding patterns, and noticed we haven't discussed/agreed on some stuff. Which is a good oportunity to just do it, now the Koha:: namespace is being filled with cool old-code rewrites with lots of test coverage.
- Transactions on heavy business functions that should definitely be wrapped inside a transaction.
Running $ cd Koha ; git grep txn is just sad.
- Little use of Exceptions in Koha. We introduced them, but still don't extend its use. Maybe related to the next item.
I think we should keep general exceptions (like Koha::Exceptions::ObjectNotFound) in the general Koha::Exceptions namespace. But move the package-specific ones into its own file. To aid maintenance. A good candidate are Koha::Exceptions::Virtualshelves
- We don't use a Try/Catch library. It would make it easier to use Koha::Exceptions with a nice syntax.
There are a couple interesting libraries: Try::Tiny and TryCatch. [1] I prefer the latter because the type check seems nicer to write and eval (instead of an if/ifelse cascade checking ref($exception) eq 'Something'.
I have my opinions on this items, and it would be great to discuss it openly. There's code in bugzilla waiting for QA, and it'd be great to have some consensus on how to organize the code for the future.
Regards
[1] http://search.cpan.org/~ether/Try-Tiny-0.27/lib/Try/Tiny.pm and http://search.cpan.org/~ash/TryCatch-1.003002/lib/TryCatch.pm
-- Tomás Cohen Arazi Theke Solutions (https://theke.io) ✆ +54 9351 3513384 GPG: B2F3C15F
_______________________________________________ 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/