Index: trunk/doc/misc/perlCodeConventions.tex =================================================================== --- trunk/doc/misc/perlCodeConventions.tex (revision 2541) +++ trunk/doc/misc/perlCodeConventions.tex (revision 2591) @@ -1,3 +1,3 @@ -%%% $Id: perlCodeConventions.tex,v 1.41 2004-11-30 22:42:30 jhoblitt Exp $ +%%% $Id: perlCodeConventions.tex,v 1.42 2004-12-01 22:34:22 jhoblitt Exp $ \documentclass[panstarrs]{panstarrs} @@ -25,5 +25,5 @@ DR00& 2004-11-12 & First Draft\\ \hline -DR01& 2004-11-24 & Second Draft\\ +DR01& 2004-12-01 & Second Draft\\ \RevisionsEnd @@ -46,6 +46,8 @@ \pagenumbering{arabic} + +%------------------------------------------------------------------------------ + \section{Introduction} - \subsection{Ancestry} @@ -60,5 +62,5 @@ CODE CONVENTIONS FOR THE JAVA^TM PROGRAMMING LANGUAGE. Copyright 1995-1999 Sun - Microsysytems, Inc. All rights reserved. + Microsystems, Inc. All rights reserved. \end{verbatim} @@ -87,5 +89,5 @@ \subsection{Conformance} -If any of the conventions estiblished in this document conflict with source +If any of the conventions established in this document conflict with source code that has been formated by \code{perltidy} (see \S\ref{perltidy}), the formatting as output by \code{perltidy} will be considered to be in compliance @@ -98,4 +100,5 @@ \code{}. + %------------------------------------------------------------------------------ @@ -126,6 +129,7 @@ \hline \file{Makefile.PL}& - The preferred name for the file that calls \code{ExtUtils::MakeMaker} or -\code{Module::Build} to create the makefile for Swig generated C code.\\ +The preferred name for the file that invokes a ``build'' process. Typically +this calls \code{ExtUtils::MakeMaker} or \code{Module::Build} setup the the +necessary files to install a module or compile Swig generated C code.\\ \end{tabular} \end{center} @@ -136,18 +140,26 @@ \section{File Organization} +General rules: + +\begin{itemize} +\item A file consists of sections that should be separated by blank lines and an optional comment identifying each section. +\item Files longer than 1000 lines are cumbersome and should be avoided. +\item For an example of a properly formatted program, see -Source File Example (\S\ref{SourceExample}). - +Complete Program Example (\S\ref{SourceExample}). + +\item ``Scripts'' have user documentation included at the end of the file. +\item ``Modules'' have documentation included in a companion \code{.pod}. +\end{itemize} \subsection{Source Files} - \subsubsection{Perl Scripts} @@ -156,5 +168,5 @@ \begin{itemize} \item -A Bourne shell ``she-bang'' for perl. \code{#!/usr/bin/perl}. +A Bourne shell ``she-bang'' for \code{perl}. \code{#!/usr/bin/perl}. \item @@ -163,8 +175,8 @@ \item -An optional minimum perl version requirement in long form (vStrings are not -allowed). Where \code(requirement = revision + version / 1000 + subversion / -1\_000\_000). E.g. A minimum requirement of Perl 5.8.5 would be expressed as -\code{use 5.008005} +An optional minimum \code{perl} version requirement in long form (vStrings are +not allowed). Where \code(requirement = revision + version / 1000 + subversion +/ 1\_000\_000). e.g. A minimum requirement of \code{perl} 5.8.5 would be +expressed as \code{use 5.008005} \item @@ -182,5 +194,5 @@ \item -Any script level "globals" declared as lexical variables. E.g. \code{my $foo} +Any script level "globals" declared as lexical variables. e.g. \code{my $foo} \item @@ -188,5 +200,5 @@ \item -A \code{__END__} token to instruct the perl parser to stop looking for +A \code{__END__} token to instruct the \code{perl} parser to stop looking for executable code. @@ -195,13 +207,10 @@ \end{itemize} -e.g. for file \file{foo.pl}: - -\verbatiminput{foo.pl} +For an example of a properly organized script, see Script File Example +(\S\ref{foo.pl}). \subsubsection{Perl Modules} -Perl Module/Namespaces deviate slightly from the standard naming conventions in -that the first character is always capitalized. All Pan-STARRS specific -modules should be under the top level \code{PS::} namespace. +Module files should have the following order: \begin{itemize} @@ -214,8 +223,8 @@ \item -An optional minimum perl version requirement in long form (vStrings are not allowed). -Where \code(requirement = revision + version / 1000 + subversion / 1\_000\_000). -E.g. A minimum requirement of Perl 5.8.5 would be expressed as \code{use -5.008005} +An optional minimum \code{perl} version requirement in long form (vStrings are +not allowed). Where \code(requirement = revision + version / 1000 + subversion +/ 1\_000\_000). e.g. A minimum requirement of \code{perl} 5.8.5 would be +expressed as \code{use 5.008005} \item @@ -235,6 +244,8 @@ \item -Any script level ``globals'' declared as lexical variables. E.g. \code{my -$foo} +Any package variables. e.g. \code{our $foo} + +\item +Any package scoped lexical variables. e.g. \code{my $foo} \item @@ -248,20 +259,19 @@ A \code{__END__} token to instruct the perl parser to stop looking for executable code. + \end{itemize} -e.g. for file \file{Foo.pm}: - -\verbatiminput{Foo.pm} +For an example of a properly organized module, see Module File Example +(\S\ref{Foo.pm}). \subsubsection{Plain Old Documentation} \label{pod} -This is the format for documentation in a \code{.pl} file after the -\code{__END__} token and the \code{.pod} file that should accompany a module -(\code{.pm}). - -e.g. for file Foo.pod: - -\verbatiminput{Foo.pod} +This is the format for documentation in a \code{.pod} file that should +accompany a module. + +For an example of a properly organized Pod, see Pod File Example +(\S\ref{Foo.pod}). + %------------------------------------------------------------------------------ @@ -269,7 +279,5 @@ \section{Indentation} -Four spaces should be used as the unit of indentation. Spaces are -recommended over tabs for indentation, but if tabs are used, the tabs -stops must be set to 8 spaces, not 4. +Four spaces should be used as the unit of indentation; tabs are forbidden. \subsection{Line Length} @@ -367,5 +375,5 @@ When deactivating 3 or more lines of code an \code{if} statement should be used with a false boolean value. The opening and closing of the \code{if} statement -should be nest to the same depth as the surrounding code and the deactivated +should be nested to the same depth as the surrounding code and the deactivated section should be indented. @@ -380,28 +388,34 @@ \end{verbatim} -Comments should be used to give overviews of code and provide -additional information that is not readily available in the code -itself. Comments should contain only information that is relevant to -reading and understanding the program. For example, information about -how the corresponding package is built or in what directory it resides -should not be included as a comment. - -Discussion of nontrivial or non-obvious design decisions is -appropriate, but avoid duplicating information that is present in (and -clear from) the code. It is too easy for redundant comments to get out -of date. In general, avoid any comments that are likely to get out of -date as the code evolves. - -Note: The frequency of comments sometimes reflects poor quality of -code. When you feel compelled to add a comment, consider rewriting the -code to make it clearer. - +\subsection{Implementation Comments} + +General rules: + +\begin{itemize} +\item +Comments should be used to give overviews of code and provide additional +information that is not readily available in the code itself. Comments should +contain only information that is relevant to reading and understanding the +program. For example, information about how the corresponding package is built +or in what directory it resides should not be included as a comment. + +\item +Discussion of nontrivial or non-obvious design decisions is appropriate, but +avoid duplicating information that is present in (and clear from) the code. It +is too easy for redundant comments to get out of date. In general, avoid any +comments that are likely to get out of date as the code evolves. + +\item Comments should not be enclosed in large boxes drawn with hashes or other characters. -\hfil\break + +\item Comments should never include special characters such as form-feed and backspace. - -\subsection{Implementation Comment Formats} +\end{itemize} + +\emph{The frequency of comments sometimes reflects poor quality of code. When +you feel compelled to add a comment, consider rewriting the code to make it +clearer.} Programs can have three styles of implementation comments: single-line, @@ -455,9 +469,8 @@ additional spaces to cause trailing comments in adjoining lines to be vertically aligned. Until such time as \code{perltidy} can following this -convention or a replacement tool is located, code as formatted by +convention or a replacement tool is located, trailing comments as formatted by \code{perltidy} will be considered acceptable.} \subsection{API Comments} -\label{DocComments} Plain Old Documentation format or Pod will be used to produce documentation of @@ -496,4 +509,5 @@ \begin{verbatim} my ($verbose, $help); # Correct when the meaning is clear + my ($foo, $bar) = @_; # Correct in a subroutine \end{verbatim} @@ -502,4 +516,76 @@ \begin{verbatim} my ($foo, @fooArray); # AVOID! +\end{verbatim} + +\subsubsection{Scalars} + +Do not initialize more then one scalar per declaration. + +\begin{verbatim} + my $foo = \$bar; # Correct + my ($foo, $bar) = (1, 2); # AVOID! +\end{verbatim} + +\subsubsection{Arrays} + +When assigning a large number of elements to an array a single column table +format should be used. + +\begin{verbatim} + my @foo = (1, 'two', 3); # Correct + + my @foo = ( # Correct + 1, + 'two', + 3, + 'four', + 5, + 'six', + 7, + 'eight', + 9, + 'ten' + ); + + my @foo = (1, 'two', 3, 'four', 5, 'six', ... # AVOID! +\end{verbatim} + +Consider using the \code{qw} operator when assigning strings to an array. + +\begin{verbatim} + my @foo = qw( 1 two 3 ); + + my @foo = qw( + 1 + two + 3 + four + 5 + six + ); +\end{verbatim} + +\subsubsection{Hashes} + +When assigning a large number of elements to a hash a double column table +format should be used. + +\begin{verbatim} + my %foo = (one => 1, two => 2); # Correct + + my %foo = ( # Correct + one => 1, + two => 2, + three => 3, + four => 4, + five => 5, + six => 6, + seven => 7, + eight => 8, + nine => 9, + ten => 10 + ); + + my %foo = (one => 1, two => 2, three => 3, ... # AVOID! \end{verbatim} @@ -520,17 +606,29 @@ \end{verbatim} -Note: This restriction does not apply to Perl's built in variables. +\emph{This restriction does not apply to Perl's built in variables.} + +\subsection{Package Variables} + +See When to Make Symbols Global (\S\ref{globals}). \subsection{Initialization} +General rules: + +\begin{itemize} +\item Try to initialize local variables where they're declared. +\item Do not initialize variables with an empty string or list - this is pointless. +\item A reason not to initialize a variable where it's declared is if the initial value depends on some computation occurring first. -In some cases it may be necessary to initialize a variable to suppress -a compiler warning; in this case a comment should explain the circumstances. +\item +In some cases it may be necessary to initialize a variable to suppress a +compiler warning; in this case a comment should explain the circumstances. +\end{itemize} \subsection{Placement} @@ -543,5 +641,5 @@ \subsection{Subroutine Declarations} -When coding Perl subroutines the following formatting rules should be +When coding subroutines the following formatting rules should be followed: @@ -558,5 +656,5 @@ \item The first line after the opening brace should declare a list of lexical -variables for input parameters and assign \code{@_} to it. +variables for input parameters and assign \code{shift} or \code{@_} to it. \item @@ -568,15 +666,29 @@ \item - +Do not directly use the \code{@_} array. + +\item Do not nest named subroutines. \end{itemize} \begin{verbatim} - sub subName + sub subName # Correct + { + my $param = shift; + ... + return $val; + } + + sub subName # Correct { my ($param1, $param2, ...) = @_; ... - return $val; + } + + sub subName # AVOID! + { + if ($_[0] eq "foo") { + ... } \end{verbatim} @@ -626,15 +738,13 @@ return undef; # Correct return; # AVOID! - - return $myDisk->size; - - return ($size ? $size : $defaultSize); -\end{verbatim} - -Note: In the absence of an explicit return statement Perl will return the + return $myDisk->size; # Correct + return ($size ? $size : $defaultSize); # Correct +\end{verbatim} + +\emph{In the absence of an explicit return statement Perl will return the results of the last statement executed. A null \code{return} statement also causes the result of the last statement executed to be returned. Relying on this behavior is likely to confuse maintenance programmers and therefore should -be avoided. +be avoided.} \subsection{if, if-else, if-elsif-else Statements} @@ -766,5 +876,6 @@ Two blank lines should always be used in the following circumstances: \begin{itemize} -\item Between sections of a source file +\item +Between sections of a source file \end{itemize} @@ -772,7 +883,9 @@ \begin{itemize} -\item Between the local variables in a subroutine and its first statement - -\item Between logical sections inside a subroutine to improve readability +\item +Between the local variables in a subroutine and its first statement + +\item +Between logical sections inside a subroutine to improve readability \end{itemize} @@ -790,9 +903,13 @@ \end{verbatim} -Note that a blank space should not be used between a subroutine name and its -opening parenthesis. This helps to distinguish keywords from subroutine calls. - -\item -A blank space should appear after commas in argument lists. +\emph{A blank space should not be used between a subroutine name and its +opening parenthesis. This helps to distinguish keywords from subroutine calls.} + +\item +A blank space should appear after commas in parameter lists. + +\begin{verbatim} + subName(1, 2, 3); +\end{verbatim} \item @@ -828,4 +945,17 @@ \end{itemize} +Blank spaces should \textit{not} be used in the following circumstances: + +\begin{itemize} +\item +A dereferencing or method call should not have any blank spaces around the +\code{->} operator. + +\begin{verbatim} + $myHashRef->{'foo'}; + $myObject->foo; +\end{verbatim} + +\end{itemize} %------------------------------------------------------------------------------ @@ -843,4 +973,42 @@ \textbf{Rules for Naming} & \textbf{Examples} \\ +\hline\\ + +Script Files & + +The filenames of all scripts should be verbal phrases, in mixed case. The +capitalized \code{.PL} suffix is reserved for ``build'' scripts. + +& + +\code{foo.pl}\hfil\break +\code{fooBar.pl}\hfil\break +\code{Makefile.PL}\hfil\break + +\\ + +Module Files & + +The filenames of all modules should be verbal phrases, in mixed case, with the +first letter capitalized. + +& + +\code{Foo.pm}\hfil\break +\code{FooBar.pm}\hfil\break + +\\ + +Pod Files & + +The filenames of all stand-alone Pod should follow the same rules as module +files. + +& + +\code{Foo.pod}\hfil\break +\code{FooBar.pod}\hfil\break + +\\ Subroutines & @@ -873,7 +1041,7 @@ \code{$i;}\hfil\break +\code{$i;}\hfil\break \code{$c;}\hfil\break \code{$myWidth;}\hfil\break -\hfil\break \code{$pseudo;}\hfil\break \code{$myFiddleFactor;}\hfil\break @@ -884,11 +1052,12 @@ Constant names should be in all capital letters and highly descriptive. The -\code{constant} pragma should be used to declare constants. Note: `constants' -created this way can not be interpolated inside of double quoted strings. +\code{constant} pragma should be used to declare constants. +\emph{``constants'' created this way can not be interpolated inside of double +quoted strings.} & \code{use constant PI => 3.14159265;}\hfil\break -\code{use constant LENGTHS => qw(2, 4, 8);}\hfil\break -\code{use constant CLASSES => (a => 1, b => 2);}\hfil\break +\code{use constant LENGTHS => qw( 2, 4, 8 );}\hfil\break +\code{use constant CLASSES => {a => 1, b => 2};}\hfil\break \\ @@ -901,7 +1070,8 @@ & -\code{package PS::Metadata;}\hfil\break -\code{package PS::Modules::Debias;}\hfil\break -\code{package PS::Modules::FlatField;}\hfil\break +\code{package PS::IPP::Metadata;}\hfil\break +\code{package PS::IPP::Modules::Debias;}\hfil\break +\code{package PS::MOPS::DB::Detections;}\hfil\break +\code{package PS::MOPS::Util::MPC;}\hfil\break \\ @@ -929,4 +1099,5 @@ \subsection{When to Make Symbols Global} +\label{globals} Only declare variables as package variables (\code{our}) if they are intended @@ -936,10 +1107,8 @@ \begin{verbatim} our $VERSION = '1.00'; # Correct + our $localCounter; # AVOID! - should be lexical use base qw( Exporter ); - our @EXPORT_OK = qw( $foo $bar ); # Correct - - our $localCounter; # AVOID! \end{verbatim} @@ -1079,5 +1248,5 @@ \subsubsection{FileHandles} -Don't use ``bare-words'' as filehandles except for \code{STDIN}, \code{STDOUT}, +Do not use ``bare-words'' as filehandles except for \code{STDIN}, \code{STDOUT}, and \code{STDERR}. @@ -1085,5 +1254,5 @@ open(STDOUT, '>', $foo) or die "can not open STDOUT: $!"; # Correct open(my $fh, '>', $foo) or die "can not open file $foo: $!"; # Correct - open(FILE , '>', $foo) or die "can not open file $foo: $!"; # AVOID! + open(FILE, '>', $foo) or die "can not open file $foo: $!"; # AVOID! \end{verbatim} @@ -1100,13 +1269,11 @@ \subsubsection{Chop vs. Chomp} -Don't use \code{chop} to remove newline characters as it will remove any +Do not use \code{chop} to remove newline characters as it will remove any character (newline or not). \begin{verbatim} my $foo = "baz\n"; # string with newline - chomp($foo); # removes "\n" - Correct chomp($foo); # chomping the same string twice is safe - chop($foo); # removes "z" - AVOID! \end{verbatim} @@ -1125,6 +1292,6 @@ Use ``cuddled'' brackets \CODE.{}. when accessing a hash element. It is -acceptable to use ``bare-words'' as a hash key (Note: this rule may be subject -to change). +acceptable to use ``bare-words'' as a hash key (\emph{this rule may be subject +to change}). \begin{verbatim} @@ -1147,5 +1314,5 @@ \end{verbatim} -Don't use \code{require} to prevent a module from exporting symbols into the +Do not use \code{require} to prevent a module from exporting symbols into the caller's namespace. Instead, provide \code{use} with an empty parameter list. @@ -1164,5 +1331,5 @@ \subsubsection{Inheritance} -Don't use the \code{@ISA} package variable for subclassing. Instead, use the +Do not use the \code{@ISA} package variable for subclassing. Instead, use the \code{base} pragma. @@ -1177,5 +1344,5 @@ ``bare-word''. Instead, use empty parentheses adjoined to the subroutine call. -\code{subs} pragma is a last resort. +\emph{Only use the \code{subs} pragma as a last resort.} \begin{verbatim} @@ -1187,4 +1354,75 @@ \end{verbatim} +\subsubsection{Calling Methods} + +Leave the parentheses off method calls that aren't being passed parameters. + +\begin{verbatim} + $myObj->fooBar; # Correct + $myObj->fooBar($foo); # Correct + $myObj->fooBar(); # AVOID! +\end{verbatim} + +\subsubsection{Quoting Strings} + +\begin{itemize} +\item +Unless variable interpolation needs to occur inside of a string or is likely to +be needed in the future, use single quotes \code{''} instead of double quotes +\code{""}. + +\item +When possible use \code{END} as the termination token for ``\code{< { type => SCALAR }, - month => { type => SCALAR, default => 1, - callbacks => - { 'is between 1 and 12' => - sub { $_[0] >= 1 && $_[0] <= 12 } - }, - }, - day => { type => SCALAR, default => 1, - callbacks => - { 'is a possible valid day of month' => - sub { $_[0] >= 1 && $_[0] <= 31 } - }, - }, - hour => { type => SCALAR, default => 0, - callbacks => - { 'is between 0 and 23' => - sub { $_[0] >= 0 && $_[0] <= 23 }, - }, - }, - minute => { type => SCALAR, default => 0, - callbacks => - { 'is between 0 and 59' => - sub { $_[0] >= 0 && $_[0] <= 59 }, - }, - }, - second => { type => SCALAR, default => 0, - callbacks => - { 'is between 0 and 61' => - sub { $_[0] >= 0 && $_[0] <= 61 }, - }, - }, - nanosecond => { type => SCALAR, default => 0, - callbacks => - { 'cannot be negative' => - sub { $_[0] >= 0 }, - } - }, - locale => { type => SCALAR | OBJECT, - default => undef }, - language => { type => SCALAR | OBJECT, - optional => 1 }, - }; -\end{verbatim} - -An example of how a validation spec is used in the \code{DateTime} module. - -\begin{verbatim} - use Params::Validate qw( validate validate_pos SCALAR BOOLEAN HASHREF OBJECT ); - . - . - - sub new - { - my $class = shift; - my %p = validate( @_, $NewValidate ); - . - . +the \code{HTTP::Range}\footnote{HTTP::Range - +http://search.cpan.org/~jhoblitt/HTTP-Range/} module. + +\begin{verbatim} + my %args = validate( @_, + { + request => { + type => OBJECT, + isa => 'HTTP::Request', + }, + length => { + type => SCALAR, + callbacks => { + 'length is > 0' => sub { $_[0] > 0 }, + 'length is + integer' => sub { $_[0] =~ /^\d+$/ }, + }, + }, + segments => { + type => SCALAR, + default => 4, + callbacks => { + 'segments is > 1' => sub { $_[0] > 1 }, + 'segments is + integer' => sub { $_[0] =~ /^\d+$/ }, + 'segments is <= length' => sub { $_[0] <= $_[1]->{ 'length' } }, + }, + }, + }, + ); + \end{verbatim} @@ -1308,4 +1533,7 @@ further details. + +%------------------------------------------------------------------------------ + \bibliographystyle{plain} \bibliography{panstarrs}