CODING_CONVENTIONS

In order to maintain consistent looking code we have defined the
following conventions.  These conventions are not exhaustive and
should be changed as necessary.

- Coding Approach

* Code defensively, we don't know anything about the caller and
  cannot rely on the sanity of any arguments passed through a
  public interface.
* Functions should check their arguments. Do not rely on the
  caller having already checked for you!
* Code for the future. If a condition depends on a conclusion
  reached before it (depending on how far before it of course)
  it might make sense to recheck the assumption where you're
  going to rely on it.
* Test everything explicitly. Throw exceptions as appropriate.
* The library should only throw ClusterException to the caller.

- Scoping & Interface

* Minimize the interface!!! Don't expose what other classes don't need.
* Minimize the functionality!!! Don't add anything w/o a use case.
* DO NOT EVER make instance variables public. All instance variable
  should only be manipulated by getters and setters.
* Specifically, use getFrobLock() to obtain pointers to locks (mutexes)
  for use in AutoLock constructors, rather than taking the address of
  an instance variable, e.g. &m_frobLock.
* Use inner classes for concepts that are only used elsewhere within
  the same class. Otherwise it deserves a top-level class.
* Use friend declarations SPARINGLY, and for good reason. If it can
  be done w/o a friend declaration, it SHOULD be done w/o one.

- Class formatting

* All class names should start with capital letters, inheritance
  should be on separate lines.
* Initializer list should be separated by lines
* Functions and data should be separated by access specifiers
* Empty code blocks are together on the same line

Here is an example that illustrates all of the following

namespace clusterlib {

class ServerImpl
    : public virtual Server, 
      public virtual ClientImpl
{
  public:
    virtual Node *getMyNode() 
    {
        return dynamic_cast<Node *>(mp_node);
    }

    bool doesNothing() {}

  private:
    ServerImpl()
        : ClientImpl(NULL)
    {
        throw ClusterException("Someone called the Server "
                               "default constructor!");
    }

  private:
    Node *mp_node;
};
   

- Exceptions

Do not declare exceptions as part of the declaration of a method or
function, C++ exception declarations only add complexity and provide
no benefit.

- 64-bit vs 32-bit

* Avoid variable size entities where possible.
* Always specify the size of any integer variable, use explicit
  sized types like int32_t and uint64_t.

- Synchronization

* Locking is your responsibility. If an instance variable needs to
  be protected, scope the mutual exclusion to the shortest code path
  possible.
* Use auto-locks so that locks are released automatically under all
  circumstances.
* Code extremely defensively for synchronization problems. If it can
  occur, it will! Some synchronization problems cannot be eliminated
  in an efficient enough manner and we may decide to avoid them by
  specifying usage conventions. Example: do not unregister interests
  in a Notifyable from a different thread than the one that originally
  registered the interest.

- Identifier Names

* PLEASE do not use single letter variable names; try to make them
  function-informative. Iterator names should end with 'It', and pointer
  variable names should end with 'P'.
* Names of classes begin with an uppercase letter (for example, Canvas).
* Both member and non-member functions begin with an lowercase letter
  (for example, textStyle).
* Class data members begin with its m (for example, m_nodes for instance) and 
  mp (for example mp_parent for pointers). 
* Clusterlib objects (i.e. Node, DataDistribution, Application) should use 
  the full name in any member functions (i.e. createDataDistribution, 
  loadApplication) and as parameters or instance names use abbrieviated names 
  as follows (i.e. m_app;).
    * Application = app
    * DataDistribution = dist
    * PropertyList = propList
    * Group = group
    * Notifyable = nt
    * Node = node
    * For pointers, add 'P' to the end of the name (i.e. appP, distP)

- Control Structures

The control structures are as follows:

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

for (expression; expression; expression) {
    statements;
}

do {
    statements;
} while (expression);

while (expression) {
    statements;
}

try {
}
catch {
}

switch (expression) {
  case constant:
     statements;
      break;
  default:
}

Special note about 'switch': There should ALWAYS be a
'default', even when it is not needed at time of writing. It may
become needed later.

If you must break up a long statement that consists of '.' or '->',
make sure to indent the following line by 4 spaces.  For example,

longNamedObjectPointer->superLongNamedObjectMemberFunction()

should become:

longNamedObjectPointer->
    superLongNamedObjectMemberFunction()

- Other indentation conventions

* Indent 4 spaces at a time.
* All closing '}' appear on a separate line.
* Function and class declarations are terminated by ';' even when
  not absolutely required.
* No spaces around macro and function arguments.

- Logging and message output

No direct messages to stdout or stderr (i.e. cout, cerr, printf, etc.).
Use log4cxx for all messaging output.

If the log message fits on a single line:

LOG_ERROR(CL_LOG, "ZooKeeperException: %s", zke.what());

Otherwise, the logger, and all arguments except the format must each
be on their own line.  The format may span one or more lines.

LOG_ERROR(LOG,
          "Skipping ZK event (type: %d, state: %d, path: '%s'), "
          "because ZK passed no context",
          type,
          state,
          sPath.c_str());

This applies to all paramater lists.  For example,

for (expression; expression;
     expression) {
}

Instead do 

for (expression; 
     expression;
     expression) {
}

- File Names

C++ source files use the suffix .cc. 
C++ header files use the suffix .h.

All file names are lower case throughout even when they're
composed of multiple words. So

clusterlib.cc not clusterLib.cc or ClusterLib.cc.

- Compilation flags & code clean-ness

Use -Wall and -Werror to force all warnings to be treated as
errors. You cannot commit code with warnings.

- Documentation

Use doxygen style comments to document all client visible functions.
The format is shown in the example below.

/**
 * Get the properties for this object (if it is allowed). If
 * subclasses do not want to allow getProperties(), override it
 * and throw a clusterlib exception.
 * 
 * @param create create the properties if doesn't exist?
 * @return NULL if no properties exists for this notifyable
 */
Properties *getProperties(bool create = false);

- Testing

All new features must have unittests to check them before committing
them.  The unittests are in clusterlib/unittests.

- STL containers

* When the map container is an rvalue, do not use the [], instead use find.
* When using vector, use at() instead of [].

- Locking & Timers

* External locking APIs should also be presented uniformly with a void
  return for blocking APIs and a bool return for non-blocking APIs.  The 
  non-blocking API should have a msecTimeout parameter where -1 means 
  blocking, 0 means immediate return, and > 0 means the number of msecs
* Internal locking APIs should be presented uniformly with a single 
  blocking/non-blocking API that has a bool return and a usecTimeout where -1 
  means blocking, 0 means immediate return, and > 0 means the number of usecs
* Shared APIs may have one or both interfaces (msTimeout and usTimeout)
* In order to determine whether the interface is msecs or usecs, it will be in
  the function name (i.e. waitMsecs(int64_t msecTimeout))
* Clusterlib locks shall not be used implicitly unless during a modify 
  operation.  Reads shall not acquire locks (this is up to the user) so that 
  clients (i.e. the CLI or the GUI) have the option of not using locks.

- Explicit

* All single argument constructors or constructory that have default
  arguments such that it can be a single argument constructor must be
  'explicit' to prevent implicit construction.