<div dir="ltr"><div>Hi Marcel,</div><div><br></div><div>Sorry for the late reply.</div><div>All looks ok to me as well, only a couple of things:</div><div><br></div><div>15b: better to point to C4 than c/p code from there to Koha :) Ideally the code should be moved to Koha, then used from there. But no always a solution I guess.<br></div><div><br></div><div>15c: On 22407 comment 3 I listed the different cases of the relationship, and how we should use them.</div><div>ie. no "return unless" if not necessary, etc. I think they could be copied to the wiki as an example to follow<br></div><div><a href="https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22407#c3">https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22407#c3</a></div><div><br></div><div>And a question about 15a, what could be a good reason to not use OOP in Koha?<br></div><div><br></div><div>Thanks!</div><div>Jonathan<br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Le mer. 29 janv. 2020 à 17:24, Marcel de Rooy <<a href="mailto:M.de.Rooy@rijksmuseum.nl">M.de.Rooy@rijksmuseum.nl</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr">
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
<span>Referring to the dev meeting and bug 22407, I propose the following rewording:</span></div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
<span><br>
</span>
<div>PERL15 Using Koha::Object[s]<br>
</div>
<div>PERL15(a) Code added to the Koha:: namespace should be object-oriented unless there is a very good reason to do otherwise. In that case the reason should be documented clearly.<br>
</div>
<div>PERL15(b) Code in Koha::Object[s] should normally not refer back to C4. Obviously, this cannot be enforced harshly until the refactoring has been completed.<br>
</div>
<div>PERL15(c) Use DBIC relations to (pre)fetch a related Koha object rather than searching it explicitly. (See bug 22407)<br>
</div>
<div><br>
</div>
<div><br>
</div>
<div>PERL28 Other object oriented code guidelines (in addition to PERL15)</div>
<div>I think we need to move the Accessor and idiom for -> new rules from PERL15 to another rule. I opened rule 28 for that depending on discussing the following:<br>
</div>
<div>Class::Accessor is a great way to provide access to member variables.<br>
</div>
<div>=> My suggestion would be to remove this rule. It is true but we do not say that it is the only way to do it. (We are not even using Class::Accessor in Koha objects.)<br>
</div>
<div>Or just say PERL28(a) To provide access to member variables, there are several ways including but not limited to using Class::Accessor.<br>
</div>
<div><br>
</div>
<div>A useful idiom for the ->new() routine in object-oriented classes that do not need to process the arguments passed in as a hashref but merely need to save them for future processing<br>
</div>
<div>=> This suggestion is fine. It says use a parameter hash for sub new. We normally do that. Reworded:<br>
</div>
<div>PERL 28(b) Use a hashref as second parameter in sub new when passing arguments.<br>
</div>
<span></span><br>
</div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
Any feedback?</div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
<br>
</div>
<div style="font-family:Calibri,Arial,Helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
Marcel</div>
<div><table style="width:100%" cellspacing="0" cellpadding="0" border="0"><tbody><tr style="font-size:0px"><td style="vertical-align:top" align="left"><table style="font-size:0px" cellspacing="0" cellpadding="0" border="0"><tbody><tr style="font-size:12px;color:rgb(0,0,1);font-style:normal;font-weight:400;white-space:nowrap"><td style="vertical-align:top;font-family:Verdana" align="left"> <span style="font-family:remialcxesans;font-size:1px;color:rgb(255,255,255);line-height:1px"></span><br></td></tr><tr style="font-size:0px"><td style="vertical-align:top" align="left"><table style="font-size:0px;line-height:normal" cellspacing="0" cellpadding="0" border="0"><tbody><tr style="font-size:0px"><td style="padding:7px 0px 0px;vertical-align:top" align="left"><a href="http://www.rijksmuseum.nl/" id="gmail-m_8194144004578745563LPlnk689713" style="text-decoration:none" target="_blank"><img src="cid:17052c6d43f4d7146ed1" title="http://www.rijksmuseum.nl" alt="http://www.rijksmuseum.nl" style="width: 185px; min-width: 185px; max-width: 185px; height: 23px; min-height: 23px; max-height: 23px; font-size: 12px;" width="185" height="23" border="0"></a></td></tr></tbody></table></td></tr><tr style="font-size:0px"><td style="vertical-align:top" align="left"><table style="white-space:normal;color:rgb(0,0,1);font-size:14.67px;font-family:Verdana;font-weight:700;font-style:normal;text-align:left;line-height:20px" cellspacing="0" cellpadding="0" border="0"><tbody><tr style="font-size:12px"><td style="font-family:Verdana"><br>Museumstraat 1<br>Postbus 74888<br>1070 DN Amsterdam<br><a href="http://www.rijksmuseum.nl/" id="gmail-m_8194144004578745563LPlnk689713" title="http://www.rijksmuseum.nl" style="text-decoration:none;color:rgb(0,0,1)" target="_blank"><strong style="font-weight:700">Rijksmuseum.nl</strong></a><br><br><span style="font-style:italic">Nu te zien:<br></span><span style="text-decoration:underline"><a href="https://www.rijksmuseum.nl/nl/nachtwacht" id="gmail-m_8194144004578745563LPlnk689713" title="Operatie Nachtwacht" style="text-decoration:underline;color:rgb(0,0,1)" target="_blank"><strong style="font-weight:700">Operatie Nachtwacht</strong></a></span><br><br><span style="font-style:italic">Verwacht:</span> <br><span style="text-decoration:underline"><a href="https://www.rijksmuseum.nl/nl/caravaggio-bernini" id="gmail-m_8194144004578745563LPlnk689713" title="Caravaggio-Bernini" style="text-decoration:none;color:rgb(0,0,1)" target="_blank"><strong style="font-weight:700">Caravaggio-Bernini. Barok in Rome</strong></a></span><br><a href="https://www.rijksmuseum.nl/nl/nu-in-het-museum/tentoonstellingen-verwacht/dankzij-waller-2010-2020" id="gmail-m_8194144004578745563LPlnk689713" title="Dankzij Waller" style="text-decoration:none;color:rgb(0,0,1)" target="_blank"><strong style="font-weight:700"><span style="text-decoration:underline">Dankzij Waller 2010-2020</span></strong></a> <br><br>T/m 18 jaar gratis<br> <br> <span style="color:rgb(0,176,80)">Please think before you print</span><br></td></tr></tbody></table></td></tr></tbody></table></td></tr></tbody></table></div></div>
_______________________________________________<br>
Koha-devel mailing list<br>
<a href="mailto:Koha-devel@lists.koha-community.org" target="_blank">Koha-devel@lists.koha-community.org</a><br>
<a href="https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel" rel="noreferrer" target="_blank">https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-devel</a><br>
website : <a href="http://www.koha-community.org/" rel="noreferrer" target="_blank">http://www.koha-community.org/</a><br>
git : <a href="http://git.koha-community.org/" rel="noreferrer" target="_blank">http://git.koha-community.org/</a><br>
bugs : <a href="http://bugs.koha-community.org/" rel="noreferrer" target="_blank">http://bugs.koha-community.org/</a><br>
</blockquote></div>