Context Navigation

← Previous Changeset
Next Changeset →

Changeset 129

Timestamp:

Mar 2, 2004, 10:45:16 AM (22 years ago)

Author:

rhl

Message:

Dealt with almost all XXX issues; greatly improved emacs section

File:

: 1 edited

trunk/doc/misc/codeConventions.tex (modified) (73 diffs)

Legend:

: Unmodified
: Added
: Removed

trunk/doc/misc/codeConventions.tex

-              r128
+              r129
 \documentclass[panstarrs]{panstarrs}
-\newcommand\note[1]{{\bf #1}}
-\newcommand\tbd[1]{{\bf [#1]}}
 \title{\PS{} Code Conventions}
 …
 \newcommand{\matchBracket}[1]{}         % help emacs match properly
 \newcommand{\XXX}{{\color{red} XXX}}
+\newcommand{\XXX}[1]{{\color{red} XXX #1\hfil\break}}
 \pagenumbering{roman} \thispagestyle{empty} \maketitle
 \pagebreak \pagestyle{fancy}
 { \Large \bf Revision History}
+\textbf{\Large Revision History}
 \begin{center}
 \begin{tabular}{|p{1in}|p{1in}|p{4.5in}|}
 \hline
 {\bf Revision Number} & {\bf Release Date} & {\bf Description} \\
+\textbf{Revision Number} & \textbf{Release Date} & \textbf{Description} \\
 \hline
 %0.1 & 2003/12/08 & First draft \\
+.1 & 2004/03/01 & First draft \\
 \hline
 \end{tabular}
 \end{center}
 { \Large \bf Referenced Documents}
+\textbf{\Large Referenced Documents}
 \begin{center}
 \begin{tabular}{|p{1in}|p{2.75in}|p{2.75in}|}
 \hline
 {\bf \PS{} ID} & {\bf Title} & {\bf Author} \\
+\textbf{\PS{} ID} & \textbf{Title} & \textbf{Author} \\
 \hline
 %%% References here.
 …
-\label{L16729}
 \subsection{Acknowledgments}
 …
 copyright notice (Copyright.doc.html).
 \subsection{\label{L16730}To whom should I Complain?}
+\subsection{To whom should I Complain?}
 While we're working on this document complain to
 \code{rhl@astro.princeton.edu}.
 %------------------------------------------------------------------------------
-\label{L16732}
 \section{File Names}
-\emph{\XXX{} I don't discuss doxygen comments here.}
 This section lists commonly used file suffixes and names.
+\label{L647}
 \subsection{File Suffixes}
 …
 \end{center}
-\label{L253}
 \subsection{Common File Names}
 …
 \textbf{File Name} & \textbf{Use} \\
 \file{Makefile} &
+  The preferred name for makefiles.  We use \code{gnumake} to build our
+software.  \emph{Do we? \XXX{}} \\
+  The preferred name for makefiles.\\
 \file{README} &
 …
 particular directory. \\
 \code{TAGS} &
+\file{TAGS} &
 The index file (built by etags) used by emacs to follow cross references. \\
 \end{tabular}
 …
 %------------------------------------------------------------------------------
-\label{L3043}
 \section{File Organization}
 …
 For an example of a properly formatted program, see
+Source File Example (section \ref{L182}).
+\label{L11684}
+Source File Example (sec. \ref{SourceExample}).
 \subsection{Source Files}
-\label{Lfile_h}
 \subsubsection{Include Files}
-\emph{\XXX{} I added this}\hfil\break
 Header files should have names starting \code{ps} or
 \code{p_ps} for private interface definitions. The latter
 …
 \item
 Any typedefs.
-\XXX{} describe use of typedefs.
 \item
 …
 typedef struct {
   int nthing;                           /* number of subthings */
   int sthing[PS_NMAX];                  /* the subthings */
+    int nthing;                           /* number of subthings */
+    int sthing[PS_NMAX];                  /* the subthings */
 } psThing;
 …
 \end{verbatim}
 \label{Lfile_c}\subsubsection{C Source Files}
+\subsubsection{C Source Files}
 C Source files have the following ordering:
 …
 \end{itemize}
 \label{Lfile_i}\subsubsection{SWIG Interface Files}
 \XXX{} Write me
+\subsubsection{SWIG Interface Files}
+\XXX{Write me}
 %------------------------------------------------------------------------------
-\label{L262}
 \section{Indentation}
 Two spaces should be used as the unit of indentation. The exact
+Four spaces should be used as the unit of indentation. The exact
 construction of the indentation (spaces vs. tabs) is unspecified. Tabs
+must be set exactly every 8 spaces (not 2).
+\label{L313}
+must be set exactly every 8 spaces (not 4).
 \subsection{Line Length}
 …
 Note: Examples for use in documentation should have a shorter line length-generally no more than 70 characters.
-\label{L248}
 \subsection{Wrapping Lines}
 …
 \begin{itemize}
+\item Break after a comma.
+\item Break before an operator.
+  \emph{ \XXX{} RHL prefers to break \textit{after} an operator}, e.g.
+\item Break after a comma or operator.
 \begin{verbatim}
 if(a < b ||
    a > 2*b) {
   x = (a + b + c) +
+    x = (a + b + c) +
     sin(z);
+}
 \end{verbatim}
-not
-\begin{verbatim}
-if(a < b
-   || a > 2*b) {
-  x = (a + b + c)
-    + sin(z);
+}
-\end{verbatim}
-as the former tells the reader more directly that it's a partial
-statement.  Do we want to modify these rules?
 \item Prefer higher-level breaks to lower-level breaks.
 …
 be more formal about what this rule means).
+\item If the above rules lead to confusing code or to code that's squished up against the right margin, just indent 8 spaces instead.
+\hfil\break\emph{\XXX{} emacs actually just uses a regular 2 space indent;
+should we do the same? }
+\item If the above rules lead to confusing code or to code that's
+squished up against the right margin, just indent 8 spaces instead.
+This should be \emph{very} rare! Consider whether you should be
+e.g. factoring code into a function.
 \end{itemize}
 …
 \end{verbatim}
+Following are two examples of breaking an arithmetic expression. The first is preferred, since the break occurs outside the parenthesized expression, which is at a higher level.
+\begin{verbatim}
+longName1 = longName2 * (longName3 + longName4 - longName5)
+           + 4 * longname6; // PREFER
+longName1 = longName2 * (longName3 + longName4
+                       - longName5) + 4 * longname6; // AVOID
+\end{verbatim}
+Following are two examples of indenting function declarations. The first is the conventional case. The second would shift the second and third lines to the far right if it used conventional indentation, so instead it indents only 8 spaces.
+Following are two examples of breaking an arithmetic expression. The
+first is preferred, since the break occurs outside the parenthesized
+expression, which is at a higher level.
+\begin{verbatim}
+    longName1 = longName2*(longName3 + longName4 - longName5) +
+*longname6;                    // PREFER
+    longName1 = longName2*(longName3 + longName4 -
+                           longName5) + 4*longname6; // AVOID
+\end{verbatim}
+Following are two examples of indenting function declarations. The
+first is the conventional case. The second would shift the second and
+third lines to the far right if it used conventional indentation, so
+instead it indents only 4 spaces (note that putting the \code{anArg}
+argument on a line of its own makes this the `natural' emacs indentation).
 \begin{verbatim}
 //CONVENTIONAL INDENTATION
 …
+}
+//INDENT 8 SPACES TO AVOID VERY DEEP INDENTS
+static type honkingLongFunctionName(int anArg,
+        Object anotherArg, String yetAnotherArg,
+        Object andStillAnother)
+//INDENT 4 SPACES TO AVOID VERY DEEP INDENTS
+static type honkingLongFunctionName(
+    int anArg,
+    Object anotherArg, String yetAnotherArg,
+    Object andStillAnother)
+{
     ...
 …
 \end{verbatim}
-Line wrapping for \code{if} statements should generally use the
--space rule, since conventional (2 space) indentation makes seeing
-the body difficult. For example:
-\begin{verbatim}
-//DON'T USE THIS INDENTATION
-if ((condition1 &amp;&amp; condition2)
-    || (condition3 &amp;&amp; condition4)
-    ||!(condition5 &amp;&amp; condition6)) { //BAD WRAPS
-    doSomethingAboutIt();            //MAKE THIS LINE EASY TO MISS
+}
-//USE THIS INDENTATION INSTEAD
-if ((condition1 &amp;&amp; condition2)
-        || (condition3 &amp;&amp; condition4)
-        ||!(condition5 &amp;&amp; condition6)) {
-    doSomethingAboutIt();
+}
-//OR USE THIS
-if ((condition1 &amp;&amp; condition2) || (condition3 &amp;&amp; condition4)
-        ||!(condition5 &amp;&amp; condition6)) {
-    doSomethingAboutIt();
+}
-\end{verbatim}
 Here are three acceptable ways to format ternary expressions:
 \begin{verbatim}
 …
 %------------------------------------------------------------------------------
-\label{L385}
 \section{Comments}
 …
 \begin{verbatim}
 #if 0
   code that is not wanted just at present
+    code that is not wanted just at present
 #endif
 \end{verbatim}
 …
 #define IGNORE_RHL 1
 #if !IGNORE_RHL
   ...
+    ...
 #endif
 ...
 #if !IGNORE_RHL
   ...
+    ...
 #endif
 \end{verbatim}
 This type of \code{#define} may appear within the main body of the file.
 \end{itemize}
-\emph{\XXX{} RHL added this.  He stands by it. }
 Comments should be used to give overviews of code and provide
 …
 date as the code evolves.
 Note:The frequency of comments sometimes reflects poor quality of
+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.
 …
 backspace.
-\label{L216}
 \subsection{Implementation Comment Formats}
+Programs can have four styles of implementation comments: block, single-line, trailing, and end-of-line.
+\label{L680}
+Programs can have four styles of implementation comments: block,
+single-line, trailing, and end-of-line.
 \subsubsection{Block Comments }
+\label{ImplBlockComments}
 Block comments are used to provide descriptions of files, functions,
 …
 describe.
+\emph{\XXX{} I don't see any reason for this blank line}\hfil\break
+A block comment should be preceded by a blank line to set it apart from the rest of the code.
+\begin{verbatim}
+/*
+ * Here is a block comment.
+ */
+\end{verbatim}
+See also Documentation Comments (section \ref{L16838}).
+\label{L341}
+See also Documentation Comments (sec. \ref{DocComments}).
 \subsubsection{Single-Line Comments}
+\label{SingleLineComment}
 Short comments can appear on a single line indented to the level of
 the code that follows. If a comment can't be written in a single line,
+it should follow the \ref{L680}block comment format. A
+single-line comment should be preceded by a blank line.
+\emph{\XXX{} I don't see any reason for this blank line}\hfil\break
+Here's an
+example of a single-line comment in C code (also see \ref{L16838}''Documentation Comments''):
+\begin{verbatim}
+if (condition) {
+  /* Handle the condition. */
+  ...
+}
+\end{verbatim}
+\label{L342}
+it should follow the \ref{ImplBlockComments}block comment format.
 \subsubsection{Trailing Comments}
 …
 \begin{verbatim}
 if (a == 2) {
   return TRUE;                          /* special case */
+    return TRUE;                          /* special case */
 } else {
+  return isPrime(a);                    /* works only for odd a */
+}
+\end{verbatim}
+\label{L286}
+    return isPrime(a);                    /* works only for odd a */
+}
+\end{verbatim}
 \subsubsection{End-Of-Line Comments}
 …
 \begin{verbatim}
 if (foo &gt; 1) {
   // Do a double-flip.
   ...
+if (foo > 1) {
+    // Do a double-flip.
+    ...
 } else {
+  return false;          // Explain why here.
+}
+\end{verbatim}
+\label{L16838}
+    return false;          // Explain why here.
+}
+\end{verbatim}
 \subsection{Documentation Comments}
+\emph{\XXX{} Need references to Doxygen here.}
+\label{DocComments}
+\XXX{Need references to Doxygen here.}
 If you need to give information about a type, interface, or variable
 that isn't appropriate for doxygenation, use an implementation block
 comment (sec. \ref{L680}) or single-line comment (sec. \ref{L341})
+comment (sec. \ref{ImplBlockComments}) or single-line comment (sec. \ref{SingleLineComment})
 immediately \textit{before} the code. For example, details about the
 implementation of a type should go in in such an implementation block
 …
 %------------------------------------------------------------------------------
-\label{L2991}
 \section{Declarations}
+\emph{\XXX{} I added this}\hfil\break
+\textit{All globally-visible symbols must be declared in
+a suitable header (.h) file.}.  Global private symbols (i.e.
+those named \code{p_ps}) should not appear in public header
+files, but rather in files whose names begin \code{p_ps}.
+\label{L2992}
+All globally-visible symbols must be declared in a suitable header
+(.h) file.  Global private symbols (i.e.  those named \code{p_ps})
+should not appear in public header files, but rather in files whose
+names begin \code{p_ps}.
 \subsection{Number Per Line}
+One declaration per line is recommended since it encourages commenting. In other words,
+One declaration per line is recommended since it encourages
+commenting. In other words,
 \begin{verbatim}
 int level;                              // indentation level
 …
 Do not put different types on the same line. Example:
 \begin{verbatim}
 int foo,  fooarray[];                   //WRONG!
+int foo,  fooarray[];                   // WRONG!
 \end{verbatim}
 …
 (Note that the comments are indented to column 40).
-\label{L18761}
 \subsection{Initialization}
+Try to initialize local variables where they're declared.
+\hfil\break\emph{\XXX{} the following is invalidated by the rewrite of section 6.3}
+The only
+Try to initialize local variables where they're declared.  The only
 reason not to initialize a variable where it's declared is if the
 initial value depends on some computation occurring first.
-\hfil\break\emph{\XXX{} added by RHL}
 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.
-\label{L16817}
 \subsection{Placement}
+\hfil\break\emph{\XXX{} I've changed this}
+Variables should ordinarily be declared as soon as they
+appear in the logical flow of control through a function. This
+allows them to be initialised as they are created, and naturally
+Variables should ordinarily be declared at the top of the block in
+which they appear, unless there is some reason to declare them later.
+This allows them to be initialised as they are created, and naturally
 associates their declaration with their use.
+In particular, indexes of \code{for} loops should usually
+For example, declarations are naturally mixed with assertions:
+\begin{verbatim}
+void function(REGION *reg)                      // image of the sky
+{
+    assert (reg != NULL);
+    const int nrow = reg->nrow;
+    assert (reg->type == psType_U16);
+    const psTypeU16 *rows = reg->rows;
+    ...
+}
+\end{verbatim}
+Indexes of \code{for} loops should usually
 be declared in the \code{for} statement (the declarations appear
 in the scope of the braces):
 \begin{verbatim}
+for (int i = 0; i &lt; maxLoops; i++) {
+  ...
+}
+\end{verbatim}
+\hfil\break\emph{\XXX{} OLD VERSION}
+Put declarations at the beginning of blocks. (A block is any code
+surrounded by curly braces \CODE.{. and \CODE.}..) Don't wait to declare
+variables until their first use; it can confuse the unwary programmer
+and hamper code portability within the scope.
+\begin{verbatim}
+void myFunction(void)
+{
+  int int1 = 0;                         // beginning of function block
+  if (condition) {
+    int int2 = 0;                       // beginning of "if" block
+for (int i = 0; i < maxLoops; i++) {
     ...
+  }
+}
+\end{verbatim}
+One exception to the rule is indexes of \code{for} loops, which
+in C99 can be declared in the \code{for} statement (and which appear
+in the scope of the braces):
+\begin{verbatim}
+for (int i = 0; i &lt; maxLoops; i++) {
+  ...
+}
+\end{verbatim}
+p>
+\emph{\XXX{} Added by RHL: }\hfil\break
+Another exception is that assertions may preceed or be mixed with declarations:
+\begin{verbatim}
+void function(REGION *reg)                      // image of the sky
+{
+  assert (reg != NULL);
+  const int nrow = reg->nrow;
+  assert (reg->type == psType_U16);
+  const psTypeU16 *rows = reg->rows;
+  ...
+}
+\end{verbatim}
+Note that this allows us to declare variables that have been unpacked
+to e.g. allow dealiasing to be declared \code{const}.
+\hfil\break\emph{\XXX{} End old version }
+}
+\end{verbatim}
 Avoid local declarations that hide declarations at higher levels. For
 …
 void myFunction(void)
+{
   if (condition) {
+    if (condition) {
     int count = 0;                      // AVOID!
     ...
+  }
+  ...
+}
+\end{verbatim}
+\label{L381}
+    }
+    ...
+}
+\end{verbatim}
 \subsection{Function Declarations}
 …
 \item
 Function declarations should be preceeded by a short comment
+describing what the function does. \hfil\break\emph{\XXX{} Doxygen? }
+describing what the function does.
+\XXX{Discuss Doxygen}
 \item The function's type should appear on the same line as the
 function declaration.
-\hfil\break\emph{\XXX{} I don't like this (I put the type
-on the previous line), but MHPCC seems to use it}
 \item All arguments should be declared with their types; i.e. no
 …
 void function(void)
+{
   ...
+    ...
+}
 \end{verbatim}
 …
+               )
+{
   struct {
       int x, y;
   } work[10];
   int ivar;
   while(isspace(*descrip++)) {}         // skip white space
   descrip--;                            // one too far. Not great code...
   ...
+    struct {
+        int x, y;
+    } work[10];
+    int ivar;
+    while(isspace(*descrip++)) {}         // skip white space
+    descrip--;                            // one too far. Not great code...
+    ...
+}
 \end{verbatim}
 …
 %------------------------------------------------------------------------------
-\label{L430}
 \section{Statements}
-\label{L431}
 \subsection{Simple Statements}
+Each line should contain at most one statement. Example:
+\begin{verbatim}
+argv++;                                 // Correct
+argc--;                                 // Correct
+argv++; argc--;                         // AVOID!
+\end{verbatim}
+\label{L15395}
+Each line should usually contain only one statement. Example:
+\begin{verbatim}
+x = sqrt(x2);                           // Correct
+i++;                                    // Correct
+x = sqrt(x2); i++;                      // Avoid!
+\end{verbatim}
+An example of a reasonably two-statement line is:
+\begin{verbatim}
+argv++; argc--;
+\end{verbatim}
+where the two actions are intimately related.
 \subsection{Compound Statements}
 …
 \end{itemize}
-\label{L438}
 \subsection{return Statements}
 …
 \end{verbatim}
-\label{L449}
 \subsection{if, if-else, if else-if else Statements}
 …
 \begin{verbatim}
 if (condition) {
   statements;
+    statements;
+}
 if (condition) {
   statements;
+    statements;
 } else {
   statements;
+    statements;
+}
 if (condition) {
   statements;
+    statements;
 } else if (condition) {
   statements;
+    statements;
 } else {
+  statements;
+}
+\end{verbatim}
+Note: \code{if} statements always use braces {}. Avoid the following error-prone form:
+\begin{verbatim}
+if (condition) //AVOID! THIS OMITS THE BRACES {}!
+    statements;
+}
+\end{verbatim}
+Note: \code{if} statements always use braces {}. Avoid the following
+error-prone form:
+\begin{verbatim}
+if (condition)                          // AVOID! THIS OMITS THE BRACES {}!
     statement;
 \end{verbatim}
-\label{L454}
 \subsection{for Statements}
 …
 \begin{verbatim}
 for (initialization; condition; update) {
   statements;
+    statements;
+}
 \end{verbatim}
 An empty \code{for} statement (one in which all the work is done
 in the initialization, condition, and update clauses) should have the
 following form:
+in the initialization, condition, and update clauses) should have one of
+the following forms:
 \begin{verbatim}
 for (initialization; condition; update);
+for (initialization; condition; update) {}
 \end{verbatim}
 …
 the loop (for the update clause).
-\label{L460}
 \subsection{while Statements}
 …
 \begin{verbatim}
 while (condition) {
   statements;
+}
 \end{verbatim}
 An empty \code{while} statement should have the following form:
+    statements;
+}
+\end{verbatim}
+An empty \code{while} statement should have one of the following forms:
 \begin{verbatim}
 while (condition);
 \end{verbatim}
+\label{L465}
+while (condition) {}
+\end{verbatim}
 \subsection{do-while Statements}
 …
 \begin{verbatim}
 do {
   statements;
+    statements;
 } while (condition);
 \end{verbatim}
-\emph{\XXX{} RHL would prefer that we mandate one of the forms}:
-\begin{verbatim}
-for (initialization; condition; update) {}
-for (initialization; condition; update) continue;
-\end{verbatim}
-Ditto for \code{while} statements.  The closing ; is subtle,
-especially bearing in mind:
-\begin{verbatim}
-do {
-    ....
-while (condition);
-\end{verbatim}
-where the while really does have a closing semicolon.
-\label{L468}
 \subsection{switch Statements}
+A \code{switch} statement should have the following form:
+A \code{switch} statement should have the following form (note the
+indentation; the \code{case/default} is indented 2 spaces, the
+other statements are at the usual 4 space indent):
 \begin{verbatim}
 switch (condition) {
+case ABC:
+  statements;
+  /* falls through */
+case DEF:
+case GHI:
+  statements;
+  break;
+case XYZ:
+  statements;
+  break;
+default:
+  statements;
+  break;
+}
+\end{verbatim}
+\emph{\XXX{} I'd prefer to indent the ``case'' 1 more spaces RHL}:
+\begin{verbatim}
+switch (condition) {
+ case ABC:
+  statements;
+  /* falls through */
+ case DEF:
+ case GHI:
+  statements;
+  break;
+ case XYZ:
+  statements;
+  break;
+ default:
+  statements;
+  break;
+  case ABC:
+    statements;
+    /* falls through */
+  case DEF:
+  case GHI:
+    statements;
+    break;
+  case XYZ:
+    statements;
+    break;
+  default:
+    statements;
+    break;
+}
 \end{verbatim}
 …
 statement), add a comment where the \code{break} statement would
 normally be. This is shown in the preceding code example with the
+ \code{/* falls through */} comment. A comment is not
+required (or expected) when the fall-through is between multiple case
+statements.
+\code{/* falls through */} comment. A comment is not required (or
+expected) when the fall-through is between multiple case statements.
 Every \code{switch} statement should include a default case,
 …
 \code{case} is later (and illegally) added after the default clause.
-\emph{ \XXX{} Added by RHL}\hfil\break
 When switching on an enumerated type, if all the elements of the
 type are included in the switch a default clause should still be
 …
 %------------------------------------------------------------------------------
-\label{L475}
 \section{White Space}
-\label{L487}
 \subsection{Blank Lines}
 …
 \item Between the local variables in a function and its first statement
+\item Before a \ref{L680}block or \ref{L341}single-line comment
+\item Before a block (sec. \ref{ImplBlockComments}) or
+  single-line (sec. \ref{SingleLineComment}) comment
 \item Between logical sections inside a function to improve readability
 \end{itemize}
-\label{L682}
 \subsection{Blank Spaces}
 …
 \begin{verbatim}
 while (true) {
   ...
+    ...
+}
 \end{verbatim}
 …
 \item A blank space should appear after commas in argument lists.
+\item All binary operators except \code{.} and \code{->}
+should be separated from their operands by spaces. Blank spaces should
+never separate unary operators such as unary minus, increment (\code{++}),
+and decrement (\code{--}) from their operands. Example:
+\begin{verbatim}
+  a += c + d;
+  a = (a + b) / (c * d);
+\item
+ Low priority binary operators (\code{+} and \code{-} and below)
+should be separated from their operands by spaces; higher
+precedence operators (e.g. \code{*}, \code{/}, \code{%},
+  \code{.} and \code{->}) should not be separated by spaces.
+Blank spaces should never separate
+unary operators such as a type cast, unary minus, increment
+(\code{++}), and decrement (\code{--}) from their operands. Examples:
+\begin{verbatim}
+    a += c + d;
+    a = (a + b)/(c*d);
+  while (*d++ = *s++) {                 // Tricky way of copying to '\0'
+      n++;
+  }
+  printf("size is %d\n", foo);
+\end{verbatim}
+\hfil\break\emph{ \XXX{} I had to fix the code to make it even make sense, but it's
+still lousy code (I admit that you'll see it in K\&R, but it's a very
+s style)}
+\hfil\break\emph{\XXX{} I don't like this either.  I'd prefer to see multiplicative
+operators (i.e. * / \%) without whitespace.  E.g.}
+\begin{verbatim}
+a%x + y;
+a*(x + y);
+(a + b)/(x + y);
+    while (*d++ = *s++) {                 // Tricky way of copying until '\0'
+        n++;
+    }
+    printf("size is %d\n", foo);
 \end{verbatim}
 …
     for (expr1; expr2; expr3)
 \end{verbatim}
-\item Casts should be followed by a blank space. Examples:
-\begin{verbatim}
-    myFunc((byte) aNum, (Object) x);
-    myFunc((int) (cp + 5), ((int) (i + 3))
-                                  + 1);
-\end{verbatim}
-\hfil\break\emph{\XXX{} I really don't like this one.  A cast is a unary operator,
-and binds harder than e.g. ++. }
 \item
-\emph{\XXX{} I added this }\hfil\break
 The word \code{assert} should be treated as a keyword, and
 separated by a space from its logical expression.
 …
 %------------------------------------------------------------------------------
-\label{L367}
 \section{Naming Conventions}
 …
 The names of all externally visible functions (i.e. all those that are
 not declared \code{static}) should be verbs, in mixed case. The
+first two characters should be \code{ps}, with the first letter
 of each internal word capitalized.
+not declared \code{static}) should be verbal phrases, in mixed
+case. The first two characters should be \code{ps}, with the first
+letter of each internal word capitalized.
 Functions that are visible at global scope but are not considered part
 …
 Except for variables, all instance, class, and class constants are in
+mixed case with a lowercase first letter. Internal words start with
+capital letters. Variable names should not start with underscore \code{_}
+as these names are, under some circumstances, reserved by the
+C standard.
+mixed case with
+Variable names should not start with underscore \code{_} as these
+names are, under some circumstances, reserved by the C
+standard. Non-globally visible words should start with a lowercase
+first letter; Internal words start with capital letters.
 Variables visible at global scope should begin with \code{ps},
 or \code{p_ps} if not considered part of the public interface.
 Variables that are local to a file should \textit{not} start
+with \code{ps} (or \code{p_ps}).
+with \code{ps} (or \code{p_ps}). These variables should be Capitalised
+at the beginning of each word.
 Variable names should be short yet meaningful. The choice of a
 …
+&
+\emph{\XXX{} const is different in C from Java (and C++) so I've rewritten
+this.}\hfil\break
+Constants used to e.g. dimension arrays should be set using
+the \code{#define}; \code{const} variables should not be used, especially
+Constants used to e.g. dimension arrays should be set using the
+\code{#define}; \code{const} variables should not be used, especially
 in header files.  Symbolic values should usually be declared using
 enumerated types (although this requires more care with namespaces).
+enumerated types.
 The names of all enumerated types and C-preprocessor symbols (but not
 …
 %------------------------------------------------------------------------------
-\label{L529}
 \section{Programming Practices}
-\label{L177}
 \subsection{When to Make Symbols Global}
 …
 with its usage to make auto-variables within a function persistent).
-\label{L1255}
 \subsection{Constants}
 …
 in a \code{for} loop as counter values.
-\emph{ \XXX{} RHL relaxed this a little}
-\label{L547}
 \subsection{Variable Assignments}
 …
 row0 = col0 = 0;                        // OK
 \end{verbatim}
-\emph{\XXX{} Relaxed by RHL}
 Do not use the assignment operator in a place where it can be easily confused with the equality operator. Example:
 \begin{verbatim}
 if (c++ = d++) {                        // AVOID!
   ...
+    ...
+}
 \end{verbatim}
 …
 \begin{verbatim}
 if ((c++ = d++) != 0) {
+  ...
+}
+\end{verbatim}
+If \code{c} and \code{d} are char variables, use \code{'\0'}
+instead of \code{0}.
+\hfil\break\emph{\XXX{} \code{'$\backslash$0'} added by RHL}
+Do not use embedded assignments in an attempt to improve run-time performance. This is the job of the compiler. Example:
+    ...
+}
+\end{verbatim}
+Do not use embedded assignments in an attempt to improve run-time
+performance. This is the job of the compiler. Example:
 \begin{verbatim}
 d = (a = b + c) + r;                    // AVOID!
 …
 \end{verbatim}
-\label{L554}
 \subsection{Miscellaneous Practices}
-\label{L331}
 \subsubsection{Parentheses}
 …
 if ((a == b) && (c == d))               // RIGHT
 \end{verbatim}
-\hfil\break\emph{\XXX{} I don't agree with this; you end up with
-too many parens.  Next section is added by RHL}
 There are some famous problems with precedence in C.  In particular,
 expressions involving the combinations of \code{||} and
 \code{\&\&} should be parenthesised as should all
+\code{&&} should be fully parenthesised, as should all
 expressions containing bitwise operators:
 \begin{verbatim}
 if ((a == b \&\& c == d) || e == f)
+if ((a == b && c == d) || e == f)
 l << (j + k)
 (l << j) + k
 …
 \end{verbatim}
-\label{L333}
 \subsubsection{Returning Values}
 …
 \begin{verbatim}
 if (booleanExpression) {
   return true;
+    return true;
 } else {
   return false;
+    return false;
+}
 \end{verbatim}
 …
 \begin{verbatim}
 if (condition) {
   return x;
+    return x;
+}
 return y;
 …
 \end{verbatim}
-\label{L353}
 \subsubsection{Expressions before `?' in the Conditional Operator }
 If an expression containing a binary operator appears before the \code{?} in the ternary \code{?: }operator, it should be parenthesized. Example:
 \begin{verbatim}
+(x &gt;= 0) ? x : -x;
+\end{verbatim}
+\label{L396}
+(x >= 0) ? x : -x;
+\end{verbatim}
 \subsubsection{Functions that Take No Parameters}
-\emph{\XXX{} added by RHL}
 When a function takes no arguments, it should be explicitly declared
 …
 static type myFunction(void)
+{
+  ...
+}
+\end{verbatim}
+\label{L395}
+    ...
+}
+\end{verbatim}
 \subsubsection{Special Comments}
 \begin{itemize}
 \item
 Use \code{\XXX{}} in a comment to flag something that is bogus but works.
+Use \code{XXX} in a comment to flag something that is bogus but works.
 \item
 Use \code{FIXME} to flag something that is bogus and broken.
 \emph{\XXX{} next items added by RHL}
+\XXX{Check Doxygen versions of these special comments. I could not
+find any occurrence of \code{@notused@} in the Doxygen documentation}
 \item
 Use \code{NOTREACHED} to indicate a line of code that cannot be reached,
 …
 \begin{verbatim}
 if (sqrt(x) < 0) {
   psAbort();                            // never returns
   exit(1);                              // NOTREACHED
+    psAbort();                            // never returns
+    exit(1);                              // NOTREACHED
+}
 \end{verbatim}
 …
+}
 \end{verbatim}
-\emph{\XXX{} I see some @notused@ comments in the MHPCC code. Is this
-usage documented? Should we adopt it?}
 \end{itemize}
 \subsubsection{\label{LNewDel}10.4.6     Constructors/Destructors}
-\emph{\XXX{} Added by RHL }
 Type definitions should always be accompanied by prototypes for
 …
 \subsection{\label{LConvOps}Naming Conversion Operators}
-\emph{\XXX{} Added by RHL }
 When defining a function to convert from one type to another, the
 name should be of the form \code{psOldToNew}, e.g.
 \code{psEquatorialToEcliptic}.
+\code{psEquatorialToEcliptic} (\emph{not} \code{psEquatorial2Ecliptic}).
 \subsection{\label{LArgumentOrder}Order of I/O Arguments}
-\emph{\XXX{} Added by RHL }
 Functions that assign to a variable should list that argument
 …
 \subsection{\label{LConstRestrict}Type Qualifiers (const and restrict)}
-\emph{\XXX{} Added by RHL }
 All interfaces and type definitions should use \code{const} and
 \code{restrict} wherever appropriate. For example,
 \begin{verbatim}
 typedef struct {
   int n;
   float *restrict vec;
+    int n;
+    float *restrict vec;
 } psVec;
 …
 so all code should be written to explicitly handle aliasing}.
-\emph{\XXX{} Sadly, it looks to me as if gcc 3.3 doesn't make
-serious use of \code{restrict}.  I believe that we should
-still encourage its use.}
 %------------------------------------------------------------------------------
+\label{L186}
+\appendix                               %Begin Appendices
+%------------------------------------------------------------------------------
 \section{Code Examples}
-\label{L182}
 \subsection{Source File Example}
+\label{SourceExample}
+\XXX{Need to include more of the rules, and shorten the example}
+Here's \file{psBuffer.h}:
+\begin{verbatim}
+#if !defined(PS_BUFFER_H)               /* here's the insides of psBuffer.h */
+#define PS_BUFFER_H
+#define PS_BUFSIZE 128                  // Size for I/O buffers
+typedef struct {
+    char buf[PS_BUFSIZE];               // buffer
+    int n;                              // number of bytes in buffer
+} psBuffer;
+psBuffer *psBufferNew(void);
+void psBufferDel(psBuffer *buf,         // buffer to delete
+                 int deep);             // NOTUSED. Do a deep delete
+void psBufferAppend(psBuffer *restrict buf, // psBuffer to append to
+                    const char *restrict str); // string to add
+#endif
+\end{verbatim}
+And here's the C source; \file{psUtils.h} provides \code{psAlloc/psFree}.
 \begin{verbatim}
 /*
  * This file implements an example of formatting a C
+ * This file implements an example of formatting a file of C code
+ *
  * It isn't a very good piece of code
 …
 #include <time.h>
 #include "psUtils.h"
+#define BUFSIZE 128                     // Size for I/O buffers
+typedef struct {
+  char buf[BUFSIZE];                    // buffer
+  int n;                                // number of bytes in buffer
+} Buffer;
+typedef enum { NL, CRNL, CR } NLType;
+static void addNewline(NLType type);    // add a newline to the buffer
+static Buffer *buf = NULL;              // I/O buffer
+#include "psBuffer.h"
+typedef enum { CRNL, NL } NLType;
+static char *getNewline(NLType type);   // return a suitable newline
+static psBuffer *buf = NULL;            // I/O buffer
 /*****************************************************************************/
 /*
  * Create/destroy Buffers
+ * Create/destroy psBuffers
  */
 static Buffer *bufferNew(void)
+psBuffer *psBufferNew(void)
+{
   Buffer *buf = psAlloc(sizeof(Buffer));
   buf->n = 0;
   return buf;
+}
 void bufferDel(Buffer *buf,             // buffer to delete
                int deep)                // NOTUSED. Do a deep delete
+    psBuffer *buf = psAlloc(sizeof(psBuffer));
+    buf->n = 0;
+    return buf;
+}
+void psBufferDel(psBuffer *buf,         // buffer to delete
+                 int deep)              // NOTUSED. Do a deep delete
+{
   if (buf == NULL) {
     return;
+  }
+    if (buf == NULL) {
+        return;
+    }
   if (buf->n > 0) {
     (void) fputs(buf->buf, stdout);
+  }
+    if (buf->n > 0) {
+        (void)fputs(buf->buf, stdout);
+    }
   psFree(buf);
+    psFree(buf);
+}
 /*****************************************************************************/
 /*
  * Append to a string to a Buffer
+ * Append to a string to a psBuffer
  */
 static void bufferAppend(Buffer *restrict buf, // Buffer to append to
                          const char *restrict str) // string to add
+void psBufferAppend(psBuffer *restrict buf, // psBuffer to append to
+                    const char *restrict str) // string to add
+{
   assert(str != NULL);
   const int len = strlen(str);
   if (buf->n + len >= BUFSIZE) {        // XXX Handle this better
     fprintf(stderr,"Sorry; too many bytes. Bye bye\n");
     abort();
     exit(1);                    // NOTREACHED
+  }
   (void) strcat(&buf->buf[buf->n], str);
   buf->n += len;
+    assert(str != NULL);
+    const int len = strlen(str);
+    if (buf->n + len >= PS_BUFSIZE) {   // XXX Handle this better
+        fprintf(stderr, "Sorry; too many bytes. Bye bye\n");
+        abort();
+        exit(1);                        // NOTREACHED
+    }
+    (void)strcat(&buf->buf[buf->n], str);
+    buf->n += len;
+}
 …
 int main(void)
+{
   buf = bufferNew();
   long t;                               // current time
   while(t = (long)time(NULL), t % 4 != 2) {}
+    buf = psBufferNew();
+    long t;                               // current time
+    while(t = (long)time(NULL), t%3 == 2) {}
+  if (t % 4 == 0) {
+    bufferAppend(buf, "Hello");
+  } else if (t % 4 == 1) {
+    bufferAppend(buf, "Bonjour");
+  } else {
+    bufferAppend(buf, "Aloha");
+  }
+  bufferAppend(buf, " World!");
+  addNewline(NL);
+  bufferDel(buf, 0);
+  return 0;
+    if (t%3 == 0) {
+        psBufferAppend(buf, "Hello");
+    } else if (t%3 == 1) {
+        psBufferAppend(buf, "Aloha");
+    }
+    psBufferAppend(buf, " World!");
+    psBufferAppend(buf, getNewline(NL));
+    psBufferDel(buf, 0);
+    return 0;
+}
 /*****************************************************************************/
 /*
  * Add a newline to the buffer
+ * Return a desired line terminator
  */
 static void addNewline(NLType type)             // what sort of newline?
+static char *getNewline(NLType type)    // what sort of newline?
+{
+  char *newline;                        // newline to add
+  switch (type) {
+   case NL:
+    newline = "\n";
+    break;
+   case CRNL:
+    newline = "\r\n";
+    break;
+   case CR:
+    newline = "\r";
+    break;
+   default:
+    newline = "NL";                     // FIXME
+    break;
+  }
+  bufferAppend(buf, newline);
+}
+\end{verbatim}
+    char *newline;                      // newline to add
+    switch (type) {
+      case CRNL:
+        newline = "\r\n";
+        break;
+      case NL:
+        newline = "\n";
+        break;
+      default:
+        newline = "NL";                     // FIXME
+        break;
+    }
+    return newline;
+}
+\end{verbatim}
+%------------------------------------------------------------------------------
+\section{Departures from Sun Java Coding Standards}
+Apart from changes required by our use of C99 rather than Java,
+this document differs from the original Sun-Java standard in
+two sorts of ways; additions and changes.
+\subsection{ Additions to the Sun-Java standards}
+\begin{itemize}
+\item
+    Naming convention for include files
+\item
+  Added naming conventions for constructors, destructors, and conversion
+  functions.
+\item
+  Specified that the `output' argument should come \emph{first}.
+\item
+  Added conventional comments for unused arguments and unreachable
+  statements (conforming to Doxygen's conventions).
+\item
+  Specified that functions taking no arguments should be explicitly
+  specified as \code{(void)} (avoiding complaints on some compilers,
+  e.g. on SGIs).
+\end{itemize}
+\subsection{Departures from the Sun-Java standards}
+\begin{itemize}
+\item
+  We break lines \emph{after} (not before) an operator.
+\item
+  We don't specify any special indentation for continued logical
+  expressions within an \code{if} clause.
+\item
+  Specify that \code{case/break} statements should be indented
+  by half an indent (2 spaces)
+\item
+  Comments need not be preceeded by a blank line
+\item
+  The restriction on only declaring variables at the top of blocks (including
+  \code{for} loops has been somewhat relaxed.
+\item
+  Relaxed the `only one statement per line' rule a little.
+\item
+  High-precedence binary operators (\code{*}, \code{/}, \code{%} and above)
+    should \emph{not} be surrounded by whitespace.
+\end{itemize}
 %------------------------------------------------------------------------------
 …
 not part of the coding standards.}
+If you want to simply set these variables you can, see the code in
+the section on hooks (section \ref{Lhooks}) below.
+\subsection{\label{Lusing customize}Using Customize to Control Formatting}
+Add this to your \file{.emacs} file:
+\begin{verbatim}
+(custom-set-variables
+  ;; custom-set-variables was added by Custom -- don't edit or cut/paste it!
+  ;; Your init file should contain only one such instance.
+ '(c-basic-offset 2)
+ '(c-default-style (quote ((other . "k\&r"))))
+ '(c-offsets-alist (quote ((statement-case-intro . 1) (statement-case-open . 1) (case-label . 1))))
+ '(comment-column 40)
+ )
+\end{verbatim}
+If you already have a
+\code{custom-set-variables} command be careful! Or set the same values
+using the \code{Customize Emacs} entry in the \code{Options} menu,
+then follow down:
+\begin{verbatim}
+Browse Customization Groups -->
+    Programming -->
+        Languages -->
+            C
+\end{verbatim}
+(an alternative way to get to the same place is
+\begin{verbatim}
+ESCx
+customize-group
+C
+\end{verbatim}
+)
+Then set the values of
+\begin{verbatim}
+ C Basic Offset    ==> 2
+ C Default Style   ==> k\&r
+ C Offsets Alist   ==> X  statement-case-intro 1
+                       X  statement-case-open  1
+                       X  case-label           1
+\end{verbatim}
+Then in the same  \code{Customize Emacs} menu, choose
+\begin{verbatim}
+Settings Matching Regexp...
+\end{verbatim}
+then set the value of
+\begin{verbatim}
+ Comment Column    ==> 40
+\end{verbatim}
+If you want to use \code{//} style comments, see the top
+of the next section.
+\subsection{Setting variables directly via lisp hooks}
+\label{Lhooks}
+If you want to use \code{//} style comments, you may want to
+make \code{ESC;} insert them rather than the traditional
+\code{/* .. */} version.  If so, add something like this to
+your \file{.emacs} file:
+\begin{verbatim}
+(add-hook 'c-mode-hook
+The easiest way to use these conventions while writing code using
+emacs is to add these lines to your \file{.emacs} file:
+\begin{verbatim}
+(define-derived-mode panstarrs-c-mode
+  c-mode "Pan-STARRS C"
+  "Major mode for editing Pan-STARRS C source.
+          \\{panstarrs-c-mode-map}"
+  (setq c-default-style '((other . "k&r")))
+  (setq c-basic-offset 4)
+  (setq comment-column 40)
+  (add-to-list 'c-offsets-alist (cons 'statement-case-intro 2))
+  (add-to-list 'c-offsets-alist (cons 'statement-case-open 2))
+  (add-to-list 'c-offsets-alist (cons 'case-label 2))
+  )
+\end{verbatim}
+You then need to ensure that this mode is set when editing code;
+you could always type \code{ESCx panstarrs-c-mode} when you
+opened a file, but the simplest way is to add
+\begin{verbatim}
+(add-to-list 'auto-mode-alist (cons "\\.[ch]$" 'panstarrs-c-mode))
+\end{verbatim}                          % $ % match that $ for emacs
+to your \file{.emacs} file; this will use \code{panstarrs-c-mode}
+for all \file{.c} and \file{.h} files.
+If you want to use C99-style comments (i.e. \code{//} to end of line),
+add this to your \file{.emacs} file too:
+\begin{verbatim}
+(add-hook 'panstarrs-c-mode-hook
           '(lambda ()
              (setq comment-start "// ")
              (setq comment-end "")))
 \end{verbatim}
+You can get fancier, of course. For example,
+\begin{verbatim}
+(setq ps-code t)                        ;this is Pan-STARRS code
+(add-hook 'c-mode-hook
+          '(lambda ()
+             (if (and (boundp 'ps-code) ps-code)
+                 (progn
+                   (setq comment-start "// ")
+                   (setq comment-end "")))))
+\end{verbatim}
+will make emacs use \code{//} if and only if the lisp variable
+\code{ps-code} is set and non-\code{nil}.
+To entirely bypass emacs' customize menus, also add this to your
+\textit{.emacs} file (I left the \code{ps-code} test in):
+\begin{verbatim}
+(add-hook 'c-mode-hook
+          '(lambda ()
+             (if (and (boundp 'ps-code) ps-code)
+                 (progn
+                   (setq c-basic-offset 2)
+                   (setq c-default-style '((other . "k&r")))
+                   (setq comment-column 40)
+                   (let ( el (vals
+                              '((statement-case-intro . 1)
+                                (statement-case-open . 1)
+                                (case-label . 1))) )
+                     (while vals
+                       (setq el (car vals))
+                       (setq vals (cdr vals))
+                       (setq c-offsets-alist
+                             (cons el
+                                   (assoc-delete-all
+                                    (car el) c-offsets-alist))))
+                     )
+                   ))))
+\end{verbatim}
+(and yes, there must be an easier way to set that alist. But this works).
 \bibliographystyle{plain}
 \bibliography{panstarrs}

Note: See TracChangeset for help on using the changeset viewer.

Context Navigation

Changeset 129

Legend:

trunk/doc/misc/codeConventions.tex

Download in other formats: