Hi all, Had a discussion with Jonathan on bug 9032 about the use of variables and requirements for Plack. Our coding guidelines do not yet mention Plack. Should we add a PERL rule that prohibits defining lexical variables (my $var) at the outermost block (file level) ? Is it also needed to move script subroutines to module level? Other requirements? It would be beneficial if we could update the coding guidelines accordingly.. Thanks, Marcel
Le 23/09/2013 07:40, Marcel de Rooy a écrit :
Hi all,
Had a discussion with Jonathan on bug 9032 about the use of variables and requirements for Plack.
Our coding guidelines do not yet mention Plack.
Should we add a PERL rule that prohibits defining lexical variables (my $var) at the outermost block (file level) ? Definetly, yes, we should add such a rule (and, imo, it's also a good coding guideline, even outside from Plack !)
-- Paul POULAIN - BibLibre http://www.biblibre.com Free & Open Source Softwares for libraries Koha, Drupal, Piwik, Jasper
Hi, On Sun, Sep 22, 2013 at 10:40 PM, Marcel de Rooy <M.de.Rooy@rijksmuseum.nl>wrote:
Should we add a PERL rule that prohibits defining lexical variables (my $var) at the outermost block (file level) ?
-1 for scripts -- it's not necessary, and doing anything to tempt folks to use 'our' unnecessarily instead would be worse. It's sufficient to ensure that any subroutines inside the file aren't inappropriately accessing file-level lexicals, and that's easy to test by verifying that the following doesn't emit any warning messages: perl -MCGI::Compile -e 'my $sub = CGI::Compile->compile($ARGV[0])' /path/to/ script.pl How are we doing at the moment, for CGI scripts)? Not badly, actually: http://paste.koha-community.org/223 A provisional +1 for modules, where use of a package-level lexical is a signal that state may be held on to inappropriately. Running the CGI::Compile test on *.pm files can point out the most egregious examples, though. As it turns out, C4 is doing less badly than I would have expected: http://paste.koha-community.org/224 Even for modules, though usage of 'our' should be discouraged except for VERSION and the like.
Is it also needed to move script subroutines to module level?
This would have to be handled carefully -- for example, to take it to an extreme, a rule that "scripts cannot have subroutines" would be problematic. While there are undoubtedly plenty of routines that currently live in scripts that could be generalized and moved into the Koha namespace, sometimes a script really ought to have subroutines purely for the sake of good organization, even if those subroutines have no utility outside of that particular script. There are plenty of legacy scripts that never managed even plain old functional decomposition; and I would hate to see anything that would (even inadvertently) discourage appropriate use of local subroutines. The key word is "appropriate" -- an internal subroutine that doesn't touch variables that are not directly passed to it is fine, of course. Regards, Galen -- Galen Charlton Manager of Implementation Equinox Software, Inc. / The Open Source Experts email: gmc@esilibrary.com direct: +1 770-709-5581 cell: +1 404-984-4366 skype: gmcharlt web: http://www.esilibrary.com/ Supporting Koha and Evergreen: http://koha-community.org & http://evergreen-ils.org
Thanks for this thorough response!
Should we add a PERL rule that prohibits defining lexical variables (my $var) at the outermost block (file level) ?
-1 for scripts -- it's not necessary, and doing anything to tempt folks to use 'our' unnecessarily instead would be worse. It's sufficient to ensure that any subroutines inside the file aren't inappropriately accessing file-level lexicals, and that's easy to test by verifying that the following doesn't emit any warning messages: You are right. I actually meant the same :) Defining them and using these vars at a lower level. Recently submitted some code that actually did that (just lazy programming and not passing these vars as parameters).. I would opt for adding such a explicit guideline.
A provisional +1 for modules, where use of a package-level lexical is a signal that state may be held on to inappropriately. Running the CGI::Compile test on *.pm files can point out the most egregious examples, though. Another guideline here might make us more aware of doing so, or at least make author, signer or QAer perhaps think about it.. Even for modules, though usage of 'our' should be discouraged except for VERSION and the like. Third time lucky: +1
So now, do we add these coding guidelines? Perhaps under one rule about defining variables?
participants (3)
-
Galen Charlton -
Marcel de Rooy -
Paul Poulain