Coding standards

From DGC Wiki

For the most part, we use the canonical Java standards, posted on the Sun [website]. They generally make sense in the context of C++ instead of Java, excepting language specific features.

The following sections contain the parts of the Java Coding Standards that can be applied to C++ and DGC in particular. This is still a work in progress.

Code organization

Modules

A module should contain a set of code that is logically related and is typically managed by a single person. More on this later.

Filenames

For modules and program names, we use unix file conventions

• Module, program and library names should be lower case, with dashes if required (but try to avoid)
• Filenames for a class should match the capitalization for that class (eg, StateClient.cc)

For C/C++ code, use the following conventions

• C++ code should end in .cc, C++ header files should end in .hh
• C code and headers should end in .c and .h, respectively

Data Files

See where to put data files for the naming conventions.

Comments and Documentation

All modules should have the following files:

• README - quick overview of the module and its basic function, including any special notes required to use or compile
• ChangeLog - list of changes in standard GNU format
• ReleaseNotes - edit during YaM release process; describe what is different in each release
• Makefile.yam - Makefile for compiling using YaM make structure

All functions, classes and data structures should be documented using doxygen. Additional comments that describe program function should be included in the source code. Doxygen is described in the Documentation API page. Any important notes about the current Doxygen configuration should be put there. Instructions and examples on how to include Doxygen-style comments should be put here.

Command Line Arguments

All programs should use gengetopt for parsing command line options. A few standard options should be defined for all modules:

     --argfile=STRING      file containing command line arguments
     --config=STRING       configuration file  (default=`adrive.config')
     --noconfig            don't load configuration file at startup  
-S, --skynet-key=INT      skynet key  (default=`0')
-v, --verbose=INT      turn on verbose error messages (default = `0')
     --rndf==STRING     specify route network definition file
     --log-level=INT      turn on gcmodule logging

There are utility functions available to support some of these functions:

• key = skynet_findkey(argc, argv) - this will parse the command line arguments and set up the skynet key. It looks for the -S or --skynet-key flags first. If they are not found, it will look for the environment variable SKYNET_KEY. If that fails, it returns 0.
• dgcFindConfigFile(cfgfile, module) - this function will return the full path to a configuration file for a given module. It checks first in the current directory and then looks in the path given by $DGC_CONFIG_DIR/etc/module. It also searches in the current YaM root directory (by looking at the working directory name).
• dgcFindRouteFile(rtefile) - this function will parse the name of a route file and attempt to locate it in the standard locations. Route files have the format <location>_<description>.<ext> and are located in the etc/routes-<location> subdirectory. Searches in the same places as dgcFindConfigFile().

C/C++ formatting

Indentation

Every programmer can choose her own preferred number of spaces as the unit of indentation, as long as it's used consistently within each module, the only requirement is to use spaces in place of tabs. Every modern editor can be set up so that when you press TAB it inserts spaces, or it automatically indents the line (e.g. emacs).

Wrapping Lines

When an expression will not fit on a single line, break it according to these general principles:

• Break after a comma.
• Break before an operator.
• Prefer higher-level breaks to lower-level breaks.
• Align the new line with the beginning of the expression at the same level on the previous line.
• If the above rules lead to confusing code or to code that's squished up against the right margin, just indent double the normal size instead.

Here are some examples of breaking method calls:

   someMethod(longExpression1, longExpression2, longExpression3, 
           longExpression4, longExpression5);
    
   var = someMethod1(longExpression1,
                   someMethod2(longExpression2,
                           longExpression3)); 

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.

   longName1 = longName2 * (longName3 + longName4 - longName5)
              + 4 * longname6; // PREFER

   longName1 = longName2 * (longName3 + longName4
                          - longName5) + 4 * longname6; // AVOID 

Following are two examples of indenting method 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 (double-size indent).

   //CONVENTIONAL INDENTATION
   someMethod(int anArg, Object anotherArg, string yetAnotherArg,
              void* andStillAnother) {
       ...
   }

   //INDENT 8 SPACES TO AVOID VERY DEEP INDENTS
   const SomeVeryLongClassName& horkingLongMethodName(int anArg,
           void* anotherArg, string yetAnotherArg,
           void* andStillAnother) {
       ...
   }

Line wrapping for if statements should generally use the double-space rule, since conventional indentation makes seeing the body difficult. For example:

//DON'T USE THIS INDENTATION
if ((condition1 && condition2)
    || (condition3 && condition4)
    ||!(condition5 && condition6)) { //BAD WRAPS
    doSomethingAboutIt();            //MAKE THIS LINE EASY TO MISS
} 

//USE THIS INDENTATION INSTEAD
if ((condition1 && condition2)
        || (condition3 && condition4)
        ||!(condition5 && condition6)) {
    doSomethingAboutIt();
} 

//OR USE THIS
if ((condition1 && condition2) || (condition3 && condition4)
        ||!(condition5 && condition6)) {
    doSomethingAboutIt();
} 

//THIS IS EVEN BETTER IMHO --Datamino 22:42, 4 January 2007 (PST)
if ((condition1 && condition2) || (condition3 && condition4)
        ||!(condition5 && condition6))
{
    doSomethingAboutIt();
} 

Here are three acceptable ways to format ternary expressions:

alpha = (aLongBooleanExpression) ? beta : gamma;  

alpha = (aLongBooleanExpression) ? beta
                                 : gamma;  

alpha = (aLongBooleanExpression)
        ? beta 
        : gamma;

Declarations

Number Per Line

One declaration per line is recommended since it encourages commenting. In other words,

int level; // indentation level
int size;  // size of table

is preferred over

int level, size;

Do not put different types on the same line. Example:

int foo,  fooarray[]; //WRONG!

Note: The examples above use one space between the type and the identifier. Another acceptable alternative is to align the name of the variables, e.g.:

int	level;	        // indentation level
int	size;	         // size of table
Entry*	currentEntry;	 // currently selected table entry

TODO: Add some rule for declaring pointers and references? Or maybe remove everything here and let everyone do whatever they want?

Initialization

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.

Placement

Note: I don't like this rule, I prefer the rule that says "declare a variable where it is first used", as advised by Bjarne Stroustrup, creator of C++. I'd say we don't need this rule, let everybody do what they want. --Datamino 22:42, 4 January 2007 (PST)

Put declarations only at the beginning of blocks. (A block is any code surrounded by curly braces "{" and "}".) Don't wait to declare variables until their first use; it can confuse the unwary programmer and hamper code portability within the scope.

   void myMethod() {
       int int1 = 0;         // beginning of method block

       if (condition) {
           int int2 = 0;     // beginning of "if" block
           ...
       }
   }

The one exception to the rule is indexes of for loops, which in Java can be declared in the for statement:

   for (int i = 0; i < maxLoops; i++) { ... }

Avoid local declarations that hide declarations at higher levels. For example, do not declare the same variable name in an inner block:

   int count;
   ...
   myMethod() {
       if (condition) {
           int count = 0;     // AVOID!
           ...
       }
       ...
   }

Class and Interface Declarations

When coding C++ classes and interfaces, the following formatting rules should be followed:

• No space between a method name and the parenthesis "(" starting its parameter list
• Open brace "{" appears at the end of the same line as the declaration statement
• Closing brace "}" starts a line by itself indented to match its corresponding opening statement, except when it is a null statement the "}" should appear immediately after the "{"

class Sample : public Base {
    int ivar1;
    int ivar2;
 
    Sample(int i, int j) {
        ivar1 = i;
        ivar2 = j;
    }
 
    // FIXME: how to format C++ initializers? of course it wasn't specified in the java coding standards
    // This is how I like them, for no particular reason --Datamino 22:42, 4 January 2007 (PST)
    Sample(int i, int j)
        : ivar1(i), ivar2(j)
    {

    }

    int emptyMethod() {}
 
    int definedElsewhere();
 
    ...
}

• Methods are separated by a blank line

Statements

Simple Statements

Each line should contain at most one statement. Example:

   argv++;       // Correct
   argc--;       // Correct  
   argv++; argc--;       // AVOID!

Compound Statements

Compound statements are statements that contain lists of statements enclosed in braces "{ statements }". See the following sections for examples.

• The enclosed statements should be indented one more level than the compound statement.
• The opening brace should be at the end of the line that begins the compound statement; the closing brace should begin a line and be indented to the beginning of the compound statement.
• Braces are used around all statements, even single statements, when they are part of a control structure, such as a if-else or for statement. This makes it easier to add statements without accidentally introducing bugs due to forgetting to add braces.

return Statements

A return statement with a value should not use parentheses unless they make the return value more obvious in some way. Example:

return;

return myDisk.size();

return (size ? size : defaultSize);

if, if-else, if else-if else Statements

The if-else class of statements should have the following form:

if (condition) {
    statements;
}

if (condition) {
    statements;
} else {
    statements;
}

if (condition) {
    statements;
} else if (condition) {
    statements;
} else {
    statements;
}
    

Note: if statements always use braces {}. Avoid the following error-prone form:

if (condition) //AVOID! THIS OMITS THE BRACES {}!
    statement;

for Statements

A for statement should have the following form:

for (initialization; condition; update) {
    statements;
}

An empty for statement (one in which all the work is done in the initialization, condition, and update clauses) should have the following form:

for (initialization; condition; update);

When using the comma operator in the initialization or update clause of a for statement, avoid the complexity of using more than three variables. If needed, use separate statements before the for loop (for the initialization clause) or at the end of the loop (for the update clause).

while Statements

A while statement should have the following form:

while (condition) {
    statements;
}

An empty while statement should have the following form:

while (condition);

do-while Statements

A do-while statement should have the following form:

do {
    statements;
} while (condition);

switch Statements

A switch statement should have the following form:

switch (condition) {
case ABC:
    statements;
    /* falls through */

case DEF:
    statements;
    break;

case XYZ:
    statements;
    break;

default:
    statements;
    break;
}

Every time a case falls through (doesn't include a break statement), add a comment where the break statement would normally be. This is shown in the preceding code example with the /* falls through */ comment.

Every switch statement should include a default case. The break in the default case is redundant, but it prevents a fall-through error if later another case is added.

Throw and try-catch Statements

Note: do we want to use exception? If the answer is yes, this is the most sensible way to use exceptions in C++ that I know of. --Datamino 22:42, 4 January 2007 (PST)

A try-catch statement should have the following format:

try {
    statements;
} catch (const ExceptionClass& e) { // omit the const only if you really need it
    statements;
}

A throw statement should have the following format:

throw SomeException("screwed up");

Do not throw pointers to objects, like in

throw new SomeException("screwed up"); // BAD

throw the objects themselves instead, so you don't have to delete the exception in the catch block. Also, catch exceptions by reference, so the object that is thrown doesn't need to be copied, and you don't have to care about copy constructors in the exception class.

Naming conventions

Naming conventions make programs more understandable by making them easier to read. They can also give information about the function of the identifier-for example, whether it's a constant, package, or class-which can be helpful in understanding the code.

class Raster;
class ImageSprite;

run();
runFast();
getBackground();

int             i; 
char            c;
float           myWidth;

// this is the preferred way to declare a constant in C++
const int MIN_WIDTH = 4;

// this is good too, but works only for integers
enum {
    MAX_WIDTH = 999
};

// if possible at all, use const variables instead of #defines 
#define GET_THE_CPU 1

Programming practices

Providing Access to Instance and Class Variables

Don't make any instance or class variable public without good reason. Often, instance variables don't need to be explicitly set or gotten-often that happens as a side effect of method calls.

One example of appropriate public instance variables is the case where the class is essentially a data structure, with no behavior (a struct). In other words, for a struct it's appropriate to make the class's instance variables public (they are public by default).

Referring to Class Variables and Methods

Avoid using an object to access a class (static) variable or method. Use a class name instead. For example:

AClass::staticMethod();      //OK
anObject.staticMethod();    //AVOID!

Avoid Magic Numbers

Numerical constants (literals) should not be coded directly, except for -1, 0, and 1, which can appear in a for loop as counter values.

Variable Assignments

Avoid assigning several variables to the same value in a single statement. It is hard to read. Example:

fooBar.fChar = barFoo.lchar = 'c'; // AVOID!

Do not use the assignment operator in a place where it can be easily confused with the equality operator. Example:

if (c++ = d++) {        // AVOID!
    ...
}

should be written as

if ((c++ = d++) != 0) {
    ...
}

but it's better if you make a separate statement for the assignment.

Do not use embedded assignments in an attempt to improve run-time performance. This is the job of the compiler. Example:

   d = (a = b + c) + r;        // AVOID!

should be written as

   a = b + c;
   d = a + r;

Parentheses

It is generally a good idea to use parentheses liberally in expressions involving mixed operators to avoid operator precedence problems. Even if the operator precedence seems clear to you, it might not be to others-you shouldn't assume that other programmers know precedence as well as you do.

   if (a == b && c == d)     // AVOID!
   if ((a == b) && (c == d)) // RIGHT

Returning Values

Try to make the structure of your program match the intent. Example:

if (booleanExpression) {
    return true;
} else {
    return false;
}

should instead be written as

return booleanExpression;

Similarly,

if (condition) {
    return x;
}
return y;

should be written as

return (condition ? x : y);

Expressions before `?' in the Conditional Operator

Should we avoid the use of the ternary ?: operator altogether, in the interests of making code accessible for those not intimately familiar with the project (and its languages)[dom]?

If an expression containing a binary operator appears before the ? in the ternary ?: operator, it should be parenthesized. Example:

   (x >= 0) ? x : -x;

Special Comments

Use a #warning directive to flag something that is bogus but works. Use that for FIXME and TODO flags too.

Old documentation

General Principles

1. Code should be well-organized and easy to find.
2. Code should be written to be reusable, when appropriate.
3. Code should be well-documented.

Code Organization

All source code we write for the project goes in the Subversion repository on grandchallenge.caltech.edu. Instructions for checking out your own copy is at Sysadmin Subversion Using. From here, the README in the top-level directory (and in other directories) should help guide you.

A complete filesystem overview is available at Sysadmin Filesystem Overview, but this includes much more than the Subversion repository. For the repository,

1. Mature, reusable code that is vehicle-independent goes in the top-level dgc directory.
2. Code that is in the initial development stage goes in the dgc/projects.
3. Vehicle-dependent code should go in the respective directory (e.g. dgc/alice or dgc/bob).
4. A lot of code that should be vehicle-independent is still in the dgc/bob directory, and should eventually be modified to be vehicle-independent and moved to dgc. This should be done in a way so as not to break the existing code.

Code Standards

As the source tree grows bigger and bigger it's time to start following a code standard for easier readility and efficient Doxygen API generation.

All code shall follow the GNU Coding Standards with the exception that a curly brace shall not be indented. Code examples can be viewed at this page.

Source code formatting

• Source formatting:
  • Max number of characters on each line (including spaces) = 80
  • Each line must always be terminated with a newline character.
  • Each curly brace (both { and }) shall be on it's own line
  • Only use spaces for indent (no tabs).
  • Indent size is two (2) spaces.
  • Northing corresponds to X-coordinates and Easting to Y-coordinates (we use the North-East-Down coordinate convention). Therefore coordinate tuples should be in the order: (x,y) or (Northing, Easting)
• Documentation:
  • Doxygen comments starts with /*! or /** and ends with */
  • Class- and function documentation shall ALWAYS be available in the .hh file. Documentation in the .cc file is optional.
  • Put class description documentation right before the class declaration in the header file (then it'll appear as "Detailed Description" in the generated documentation).
  • Doxygen function parameters shall be documented with \param
  • Doxygen function returns shall be documented with \return
  • Any involved units (m/s, degrees, radians) must ALWAYS be documented
  • Any sections of code that should be addressed later should carry a #warning preprocessor directive to alert the attention of the user when the code is compiled. This is to be used in place of TODO and FIXME comment tags.

Debug Levels

It's a good idea to keep a DEBUG_LEVEL variable that controls the amount of debug messages that print to stderr/stdout. Example:

if(DEBUG_LEVEL > 0)
{ 
  printf("CMap UTM Coordinates      = % 12.3f % 12.3f % 12.3f % 12.3f\n",
         cmap_utm_coordinates.left, cmap_utm_coordinates.right,
         cmap_utm_coordinates.bottom, cmap_utm_coordinates.top);
}

Error reporting standards

The recommended error format for printing to stderr or stdout is

filename [line]: Error message text here - Action

Examples:

CMap.cc [238]: Coordinates are out of bounds - ignoring/continuing
CMap.hh [134]: Log file to read from not found - aborting!

Cross-listing of bugs and commits

It's a good idea to provide a URL link to a bug that is resolved in a commit message. Similarly, when closing a bug it's helpful to provide the commit revision that closes the bug.

Example code following the current standard

/*! Check if a double is within -10 and 10
 * \param  value  The double to be checked
 * \return -1 if it's below -10, 0 if it is in the range and 
 *         1 if it's above 10.
 */
int check_range(double value) 
{
  if(value < -10)
  { 
    // even if there's only one line in an if-statement, 
    // it's best practice to use curly braces anyway (as shown)
    return -1;
  }
  else if(value > 10)
  {
    return 1;
  }
  return 0;
}

Preprocessor Warnings

A great way to put notes in your code (TODO/FIXME/HACK) for future improvement is to put a preprocessor warning directive in your code. That way each time the code is compiled, you'll see the warning. For example:

#warning "could overshoot bounds here. check this code"
i++;

Makefile Standards

Makefile variables

Wherever used, CPATH, CFLAGS, CPP_FLAGS and CXXFLAGS should include only those options which are generally independent of the platform and version of gcc. Platform-dependent definitions of these variables should be specified as environment variables (set, e.g., in the user's .bashrc file using the command export), and the Makefile should append to these definitions. For example,

$ echo $CXXFLAGS
-pthread

$ grep CXXFLAGS Makefile 
CXXFLAGS+=-g -O2 

$ make
g++ -pthread -g -O2 [...]

In particular, hardware optimization compile options (e.g. CPP_FLAGS=-march=pentium4 -mcpu=pentium4) should NOT be included in the makefiles.

Compilation flags

Warnings are a very good way to get help writing good and safe code. Therefore you should have -Wall as a compilation flag to recieve all available warnings. Also do NOT use the -Wno-deprecated which currently is in many makefiles. Edit your code instead! The following flags should be added to your CPP_FLAGS variable (example: CPP_FLAGS+= -g -O2 -pthread -Wall):

• -g : to include debug info in the compiled code
• -O2 or -O3 : to set the optimization level (3 is max)
• -pthread : to enable multithreading support (required if you use Boost threading, which is very commmon in our software).
• -Wall : to display all warnings

Autotools (Proposed)

Using the GNU Autotools (autoconf, automake, libtool) has been suggested many times for the project, but nobody has yet gathered enough expertise to try to adopt these tools. They may be able to greatly streamline our code development process. Apparently, to get the full scoop on how to use these tools, you should read the GNU Autoconf, Automake, and Libtool book, available online.

TAGS (Proposed)

Using the etags/ctags programs a database can be created that stores the locations (in source files) of all variables/functions/definitions/etc. This database can easily be accessed in an editor (M-. in emacs) to go to the definition of a variable/function/etc. This can also be used to autocomplete while writing code, quickly see which args a function takes, etc. projects/planner has a "tags" makefile rule as an example. http://www.linuxjournal.com/article/0153 has a more detailed description