<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=utf-8"><meta name=Generator content="Microsoft Word 15 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
        {font-family:Helvetica;
        panose-1:2 11 6 4 2 2 2 2 2 4;}
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
        {font-family:"Segoe UI Symbol";
        panose-1:2 11 5 2 4 2 4 2 2 3;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0cm;
        margin-bottom:.0001pt;
        font-size:12.0pt;
        font-family:"Times New Roman",serif;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:blue;
        text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
        {mso-style-priority:99;
        color:purple;
        text-decoration:underline;}
span.EmailStyle17
        {mso-style-type:personal-reply;
        font-family:"Calibri",sans-serif;
        color:windowtext;}
.MsoChpDefault
        {mso-style-type:export-only;
        mso-fareast-language:EN-US;}
@page WordSection1
        {size:612.0pt 792.0pt;
        margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
        {page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body lang=EN-AU link=blue vlink=purple><div class=WordSection1><p class=MsoNormal><a name="_MailEndCompose"><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-language:EN-US'>After reviewing </span></a><a href="http://search.cpan.org/~ether/Try-Tiny-0.27/lib/Try/Tiny.pm"><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-language:EN-US'>http://search.cpan.org/~ether/Try-Tiny-0.27/lib/Try/Tiny.pm</span></a><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-language:EN-US'>, I’d add a semi-colon to the end of that sample code, since catch {} apparently is just an argument to try():<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-language:EN-US'>“catch (&;@) Intended to be used in the second argument position of try.”<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal>use Koha::Exceptions;<br>use Try::Tiny;<br>sub boom {<br>    Koha::Exceptions::Exception->throw;<br>}<br>try {<br>    boom();<br>} catch {<br>    if ( ref($_) eq 'Koha::Exceptions::Exception' ) {<br>        # Do that<br>    } elsif ( ref($_) eq 'Koha::Exceptions::AnotherException' ) {<br>        # do something else<br>    }<br>};<o:p></o:p></p><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>It’s the same gotcha that you get with eval {}; unfortunately. <span style='font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-language:EN-US'><o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-language:EN-US'>Interesting warning in the perldoc:<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-language:EN-US'>“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.”<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-language:EN-US'>I suppose it should be safe enough though since we’re providing the exception classes ourselves…<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-language:EN-US'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'>David Cook<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'>Systems Librarian<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'>Prosentient Systems<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'>72/330 Wattle St<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'>Ultimo, NSW 2007<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'>Australia<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'>Office: 02 9212 0899<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif'>Direct: 02 8005 0595<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;mso-fareast-language:EN-US'><o:p> </o:p></span></p><div style='border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt'><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0cm 0cm 0cm'><p class=MsoNormal><b><span lang=EN-US style='font-size:11.0pt;font-family:"Calibri",sans-serif'>From:</span></b><span lang=EN-US style='font-size:11.0pt;font-family:"Calibri",sans-serif'> koha-devel-bounces@lists.koha-community.org [mailto:koha-devel-bounces@lists.koha-community.org] <b>On Behalf Of </b>Tomas Cohen Arazi<br><b>Sent:</b> Thursday, 15 September 2016 1:19 AM<br><b>To:</b> Jonathan Druart <jonathan.druart@bugs.koha-community.org>; koha-devel <koha-devel@lists.koha-community.org><br><b>Subject:</b> Re: [Koha-devel] Coding patterns discussion<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><div><p class=MsoNormal style='margin-bottom:12.0pt'><o:p> </o:p></p><div><div><p class=MsoNormal>El mié., 14 sept. 2016 a las 4:58, Jonathan Druart (<<a href="mailto:jonathan.druart@bugs.koha-community.org">jonathan.druart@bugs.koha-community.org</a>>) escribió:<o:p></o:p></p></div><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm'><p class=MsoNormal>Hi,<br><br>We used to have problems with transactions, if they were used from the<br>a subroutine/method which was called from tests (which use<br>transactions as well), it did not work.<br>Two transactions could not be started together.<o:p></o:p></p></blockquote><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>I recall nested transactions were problematic.<o:p></o:p></p></div><div><p class=MsoNormal> <o:p></o:p></p></div><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm'><p class=MsoNormal>Now that we are using DBIx::Class transactions it works correctly and<br>we will be able to use it in the codebase everywhere we need it.<br>I try to use it as soon as it's useful (see<br>Koha::Acquisition::Currency->store and bug 16907<br>Koha::Patrons->delete).<o:p></o:p></p></blockquote><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>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.<o:p></o:p></p></div><div><p class=MsoNormal> <o:p></o:p></p></div><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm'><p class=MsoNormal>Regarding the try-catch module, the following is fine by me:<br>use Koha::Exceptions;<br>use Try::Tiny;<br>sub boom {<br>    Koha::Exceptions::Exception->throw;<br>}<br>try {<br>    boom();<br>} catch {<br>    if ( ref($_) eq 'Koha::Exceptions::Exception' ) {<br>        # Do that<br>    } elsif ( ref($_) eq 'Koha::Exceptions::AnotherException' ) {<br>        # do something else<br>    }<br>}<o:p></o:p></p></blockquote><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>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 <a href="http://shelves.pl">shelves.pl</a> catches them on another way, in master!<o:p></o:p></p></div><div><p class=MsoNormal>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.<o:p></o:p></p></div><div><p class=MsoNormal><o:p> </o:p></p></div><blockquote style='border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-right:0cm'><p class=MsoNormal>You should also have a look at Bug 13995 - Proper Exception handling<o:p></o:p></p></blockquote><div><p class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>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.<o:p></o:p></p></div><div><p class=MsoNormal> <o:p></o:p></p></div><div><p class=MsoNormal>Cheers <o:p></o:p></p></div></div></div><div><p class=MsoNormal>-- <o:p></o:p></p></div><div><div><div><p class=MsoNormal><span style='font-size:9.5pt;font-family:"Helvetica",sans-serif;color:#757575'>Tomás Cohen Arazi<o:p></o:p></span></p></div><div><p class=MsoNormal><span style='font-size:9.5pt;font-family:"Helvetica",sans-serif;color:#757575'>Theke Solutions (<a href="http://theke.io/">https://theke.io</a>)<br></span><span style='font-size:9.5pt;font-family:"Segoe UI Symbol",sans-serif;color:#757575'>✆</span><span style='font-size:9.5pt;font-family:"Helvetica",sans-serif;color:#757575'> +54 9351 3513384<br>GPG: B2F3C15F<o:p></o:p></span></p></div></div></div></div></div></body></html>