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 <http://theke.io/>) ✆ +54 9351 3513384 GPG: B2F3C15F
Hello, It's good opportunity to do coding jointly. Best, Kamal On Sep 13, 2016 12:32 AM, "Tomas Cohen Arazi" <tomascohen@gmail.com> wrote:
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 <http://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/
I didn’t even know Koha::Exceptions was a thing! How does it work? Wasn’t Kyle going to work on some sort of developer handbook so there would be a central repository of guidelines/patterns? I read the Koha Coding Guidelines many moons ago, but it would be nice to know when there are updates. Since I’m not as active in the community these days, I don’t know if I have much of a say over things anymore, but it would be good to be informed of updates to the guidelines, so that a person can contribute more efficiently when one does contribute. TryCatch requires Moose while Try::Tiny is lightweight, but I guess we’re going down the Moose rabbit hole anyways these days? Doesn’t TryCatch just do try/catch/catch instead of if/elsif/elsif? David Cook Systems Librarian Prosentient Systems 72/330 Wattle St Ultimo, NSW 2007 Australia Office: 02 9212 0899 Direct: 02 8005 0595 From: koha-devel-bounces@lists.koha-community.org [mailto:koha-devel-bounces@lists.koha-community.org] On Behalf Of Tomas Cohen Arazi Sent: Tuesday, 13 September 2016 4:32 AM To: koha-devel <koha-devel@lists.koha-community.org> Subject: [Koha-devel] Coding patterns discussion 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 <http://theke.io/> ) ✆ +54 9351 3513384 GPG: B2F3C15F
H David, the coding guidelines are in effect as documented on the wiki: https://wiki.koha-community.org/wiki/Coding_Guidelines Changes and additions are discussed regularly at the development meetings and documented in the meeting minutes. You can also 'watch' the wiki page to get informed whenever the page is changed. At the moment I think we don't use Moose in Koha. Hope this helps, Katrin On 13.09.2016 01:38, David Cook wrote:
I didn’t even know Koha::Exceptions was a thing! How does it work? Wasn’t Kyle going to work on some sort of developer handbook so there would be a central repository of guidelines/patterns?
I read the Koha Coding Guidelines many moons ago, but it would be nice to know when there are updates. Since I’m not as active in the community these days, I don’t know if I have much of a say over things anymore, but it would be good to be informed of updates to the guidelines, so that a person can contribute more efficiently when one does contribute.
TryCatch requires Moose while Try::Tiny is lightweight, but I guess we’re going down the Moose rabbit hole anyways these days? Doesn’t TryCatch just do try/catch/catch instead of if/elsif/elsif?
David Cook
Systems Librarian
Prosentient Systems
72/330 Wattle St
Ultimo, NSW 2007
Australia
Office: 02 9212 0899
Direct: 02 8005 0595
*From:*koha-devel-bounces@lists.koha-community.org [mailto:koha-devel-bounces@lists.koha-community.org] *On Behalf Of *Tomas Cohen Arazi *Sent:* Tuesday, 13 September 2016 4:32 AM *To:* koha-devel <koha-devel@lists.koha-community.org> *Subject:* [Koha-devel] Coding patterns discussion
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 <http://search.cpan.org/%7Eether/Try-Tiny-0.27/lib/Try/Tiny.pm> and http://search.cpan.org/~ash/TryCatch-1.003002/lib/TryCatch.pm <http://search.cpan.org/%7Eash/TryCatch-1.003002/lib/TryCatch.pm>
--
Tomás Cohen Arazi
Theke Solutions (https://theke.io <http://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/
On Mon, Sep 12, 2016 at 7:38 PM, David Cook <dcook@prosentient.com.au> wrote:
I didn’t even know Koha::Exceptions was a thing! How does it work? Wasn’t Kyle going to work on some sort of developer handbook so there would be a central repository of guidelines/patterns?
You can find it here: https://wiki.koha-community.org/wiki/Developers_Handbook It doesn't have any documentation on using Exceptions, and that would definitely be a good addition. I feel like it's a bit "messy" as I started it by using the coding guidelines as a base, but it's not a bad start! If anyone wants to work on it, please do!
I read the Koha Coding Guidelines many moons ago, but it would be nice to know when there are updates. Since I’m not as active in the community these days, I don’t know if I have much of a say over things anymore, but it would be good to be informed of updates to the guidelines, so that a person can contribute more efficiently when one does contribute.
TryCatch requires Moose while Try::Tiny is lightweight, but I guess we’re going down the Moose rabbit hole anyways these days? Doesn’t TryCatch just do try/catch/catch instead of if/elsif/elsif?
I had thought TryCatch's use of Moose would be a deal-breaker ( at least until Koha is officially plack-only ), but after inspecting the code, I don't believe it actually uses Moose. It does use packages in the Moose namespace, but it doesn't appear to use Moose itself, though it's possible I didn't go far enough down the rabbit hole. At this point I think I'm in favor of TryCatch instead of Try::Tiny do to it's ability to catch multiple types of exceptions and it's avoidance of messing with error handlers. Kyle
Hi, Il 13/09/2016 12:19, Kyle Hall ha scritto:
TryCatch requires Moose while Try::Tiny is lightweight, but I guess we’re going down the Moose rabbit hole anyways these days? Doesn’t TryCatch just do try/catch/catch instead of if/elsif/elsif?
I had thought TryCatch's use of Moose would be a deal-breaker ( at least until Koha is officially plack-only ), but after inspecting the code, I don't believe it actually uses Moose. It does use packages in the Moose namespace, but it doesn't appear to use Moose itself, though it's possible I didn't go far enough down the rabbit hole.
reading the code of TryCatch I see that it use: Moose::Util::TypeConstraints that it use: Moose::Exporter; Moose::Deprecated that they use: Moose::Util::MetaRole Moose::Util Try::Tiny So in fact: -- Installing TryCatch we install also Try::Tiny -- We need to create an ad hoc debian package, the debian standard package uses Moose. -- Try::Tiny is in this package: https://packages.debian.org/jessie/libtry-tiny-perl, version 0.22 it is enough ? Bye Zeno Tajoli -- Zeno Tajoli /SVILUPPO PRODOTTI CINECA/ - Automazione Biblioteche Email: z.tajoli@cineca.it Fax: 051/6132198 *CINECA* Consorzio Interuniversitario - Sede operativa di Segrate (MI)
According to MetaCPAN*, TryCatch has a dependency on Moose: https://metacpan.org/pod/TryCatch It's in the META.yml and Makefile: https://metacpan.org/source/ASH/TryCatch-1.003002/META.yml https://metacpan.org/source/ASH/TryCatch-1.003002/Makefile.PL So you're going to have a hard time building TryCatch without Moose, even if you build a custom Debian package, I imagine. As for the source code (https://metacpan.org/source/ASH/TryCatch-1.003002/lib/TryCatch.pm), I see Moose::Util::TypeConstraints, which uses a few Moose::* packages which Zeno has pointed out. It also requires Class::MOP, which appears to load some Moose XS with the XSLoader (https://metacpan.org/source/Class::MOP#L32). And actually... if you look at Moose::Exporter... that's part of the Moose package: https://metacpan.org/source/ETHER/Moose-2.1805, so you need to install "Moose" if you're going to get Moose::Exporter and Moose::Deprecated. If you see the TODO for TryCatch, you'll see it's on the TODO list to split out the Moose dependency: https://metacpan.org/pod/TryCatch#TODO *I always use MetaCPAN these days when I'm tracking Perl dependencies since it does a lot of autodiscovery and it's just a nicer interface than CPAN. David Cook Systems Librarian Prosentient Systems 72/330 Wattle St Ultimo, NSW 2007 Australia Office: 02 9212 0899 Direct: 02 8005 0595
-----Original Message----- From: koha-devel-bounces@lists.koha-community.org [mailto:koha-devel- bounces@lists.koha-community.org] On Behalf Of Tajoli Zeno Sent: Tuesday, 13 September 2016 9:42 PM To: koha-devel <koha-devel@lists.koha-community.org> Subject: Re: [Koha-devel] Coding patterns discussion
Hi,
Il 13/09/2016 12:19, Kyle Hall ha scritto:
TryCatch requires Moose while Try::Tiny is lightweight, but I guess we're going down the Moose rabbit hole anyways these days? Doesn't TryCatch just do try/catch/catch instead of if/elsif/elsif?
I had thought TryCatch's use of Moose would be a deal-breaker ( at least until Koha is officially plack-only ), but after inspecting the code, I don't believe it actually uses Moose. It does use packages in the Moose namespace, but it doesn't appear to use Moose itself, though it's possible I didn't go far enough down the rabbit hole.
reading the code of TryCatch I see that it use: Moose::Util::TypeConstraints that it use: Moose::Exporter; Moose::Deprecated that they use: Moose::Util::MetaRole Moose::Util Try::Tiny
So in fact: -- Installing TryCatch we install also Try::Tiny -- We need to create an ad hoc debian package, the debian standard package uses Moose. -- Try::Tiny is in this package: https://packages.debian.org/jessie/libtry-tiny-perl, version 0.22 it is enough ?
Bye Zeno Tajoli
-- Zeno Tajoli /SVILUPPO PRODOTTI CINECA/ - Automazione Biblioteche Email: z.tajoli@cineca.it Fax: 051/6132198 *CINECA* Consorzio Interuniversitario - Sede operativa di Segrate (MI) _______________________________________________ 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/
Oops, looks like we have created two things with very similar names: https://wiki.koha-community.org/wiki/Developers_Handbook https://wiki.koha-community.org/wiki/Developer_handbook The second one was started by me to try and tie together all the pages in the wiki that a developer should know. Coule we rename one of them? Best regards, Magnus On 13 September 2016 at 12:19, Kyle Hall <kyle.m.hall@gmail.com> wrote:
On Mon, Sep 12, 2016 at 7:38 PM, David Cook <dcook@prosentient.com.au> wrote:
I didn’t even know Koha::Exceptions was a thing! How does it work? Wasn’t Kyle going to work on some sort of developer handbook so there would be a central repository of guidelines/patterns?
You can find it here: https://wiki.koha-community.org/wiki/Developers_Handbook
It doesn't have any documentation on using Exceptions, and that would definitely be a good addition. I feel like it's a bit "messy" as I started it by using the coding guidelines as a base, but it's not a bad start! If anyone wants to work on it, please do!
I read the Koha Coding Guidelines many moons ago, but it would be nice to know when there are updates. Since I’m not as active in the community these days, I don’t know if I have much of a say over things anymore, but it would be good to be informed of updates to the guidelines, so that a person can contribute more efficiently when one does contribute.
TryCatch requires Moose while Try::Tiny is lightweight, but I guess we’re going down the Moose rabbit hole anyways these days? Doesn’t TryCatch just do try/catch/catch instead of if/elsif/elsif?
I had thought TryCatch's use of Moose would be a deal-breaker ( at least until Koha is officially plack-only ), but after inspecting the code, I don't believe it actually uses Moose. It does use packages in the Moose namespace, but it doesn't appear to use Moose itself, though it's possible I didn't go far enough down the rabbit hole. At this point I think I'm in favor of TryCatch instead of Try::Tiny do to it's ability to catch multiple types of exceptions and it's avoidance of messing with error handlers.
Kyle
_______________________________________________ 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/
Next dev meeting is on September 28: https://wiki.koha-community.org/wiki/Development_IRC_meeting_28_September_20... 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/
Hi Jonathan, Unfortunately, I doubt that I'll ever be able to attend a dev meeting. Australia/Sydney 21 UTC = 7am 19 UTC = 5am 7 UTC = 5pm Of course, looking at the times for others, I see that the times for Europe aren't always that desirable either. Europe/Paris 21 UTC = 11pm 19 UTC = 9pm 7 UTC = 9am Pacific/Auckland 21 UTC = 10am 19 UTC = 8am 7 UTC = 8pm America/Los_Angeles 21 UTC = 2pm 19 UTC = 12pm 7 UTC = 12am And I doubt it's worthwhile changing one of the meeting times to be inclusive of Australians, as there probably aren't that many of us. In fact, I'm sure 7am and 5pm aren't too bad for some Australians, but for me those are times where I'm with my family and not at a computer, so maybe I'm just the exception. As Katrin said, there are always the meeting minutes and adding a "watch" on the wiki page. Even if I can't contribute, I can at least keep track of what's discussed that way. David Cook Systems Librarian Prosentient Systems 72/330 Wattle St Ultimo, NSW 2007 Australia Office: 02 9212 0899 Direct: 02 8005 0595
-----Original Message----- From: koha-devel-bounces@lists.koha-community.org [mailto:koha-devel- bounces@lists.koha-community.org] On Behalf Of Jonathan Druart Sent: Tuesday, 13 September 2016 5:19 PM To: koha-devel <koha-devel@lists.koha-community.org> Subject: Re: [Koha-devel] Coding patterns discussion
Next dev meeting is on September 28: https://wiki.koha- community.org/wiki/Development_IRC_meeting_28_September_2016
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/
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/
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/
El mié., 14 sept. 2016 a las 4:58, Jonathan Druart (< jonathan.druart@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
After reviewing <http://search.cpan.org/~ether/Try-Tiny-0.27/lib/Try/Tiny.pm> http://search.cpan.org/~ether/Try-Tiny-0.27/lib/Try/Tiny.pm, I’d add a semi-colon to the end of that sample code, since catch {} apparently is just an argument to try(): “catch (&;@) Intended to be used in the second argument position of try.” 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 } }; It’s the same gotcha that you get with eval {}; unfortunately. Interesting warning in the perldoc: “The value of $_ in the catch block is not guaranteed to be the value of the exception thrown ($@) in the try block. There is no safe way to ensure this, since eval may be used unhygienically in destructors. The only guarantee is that the catch will be called if an exception is thrown.” I suppose it should be safe enough though since we’re providing the exception classes ourselves… David Cook Systems Librarian Prosentient Systems 72/330 Wattle St Ultimo, NSW 2007 Australia Office: 02 9212 0899 Direct: 02 8005 0595 From: koha-devel-bounces@lists.koha-community.org [mailto:koha-devel-bounces@lists.koha-community.org] On Behalf Of Tomas Cohen Arazi Sent: Thursday, 15 September 2016 1:19 AM To: Jonathan Druart <jonathan.druart@bugs.koha-community.org>; koha-devel <koha-devel@lists.koha-community.org> Subject: Re: [Koha-devel] Coding patterns discussion El mié., 14 sept. 2016 a las 4:58, Jonathan Druart (<jonathan.druart@bugs.koha-community.org <mailto:jonathan.druart@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 <http://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
participants (8)
-
David Cook -
Jonathan Druart -
Kamal Hossain -
Katrin -
Kyle Hall -
Magnus Enger -
Tajoli Zeno -
Tomas Cohen Arazi