rules.org

#+MACRO: version 0.6
#+LaTeX_HEADER: \def\rulesversion{0.6}
#+TITLE: ATLAS C++ coding guidelines, version {{{version}}}
#+AUTHOR: Shaun Roe (CERN), Scott Snyder (BNL), and the former ATLAS Quality Control group
#+EMAIL: Correspondence to snyder@bnl.gov.
# Checked with org-mode 8.2.10 (as bundled with emacs 25.2.1)

# Along with the change to org-export-dictionary below, this changes
# the `Footnotes' section in HTML to `References'
#+LANGUAGE: en-references

#+TWIKI_HEADER: *Do not edit this page.  Its contents are generated automatically from the source at [[http://gitlab.cern.ch/ssnyder/coding-rules]].*

# Put a frame around examples in LaTeX.
#+LaTeX_HEADER: \usepackage{fancyvrb}
#+LaTeX_HEADER: \RecustomVerbatimEnvironment{verbatim}{Verbatim}{frame=single}

# For LaTeX, change footnotes to endnotes, and format markers like `[n]'.
# In HTML, the marker style is changed with the org-export-html-footnote-format
# setting below.
#+LaTeX_HEADER: \usepackage{endnotes}
#+LaTeX_HEADER: \let\footnote=\endnote
#+LaTeX_HEADER: \def\makeenmark{[\theenmark]}
#+LaTeX_HEADER: \def\notesname{References}
#+LaTeX_HEADER: \let\enoteformatsave=\enoteformat
#+LaTeX_HEADER: \def\enoteformat{\enoteformatsave~}
#+LaTeX_HEADER: \makeatletter
#+LaTeX_HEADER: \def\@makefnmark{[\@thefnmark]}
#+LaTeX_HEADER: \makeatother

#+LaTeX_HEADER: \usepackage{lineno}
#+LaTeX_HEADER: \linenumbers

#+LaTeX_HEADER: \usepackage{fancyhdr}
#+LaTeX_HEADER: \pagestyle{fancy}
#+LaTeX_HEADER: \rfoot{Version \rulesversion}

#+LaTeX_HEADER: \def\mylabel#1{\label{#1}#1}


* Introduction

This note gives a set of guidelines and recommendations for coding
in C++ for the ATLAS experiment.

There are several reasons for maintaining and following a set of programming
guidelines.  First, by following some rules, one can avoid some
common errors and pitfalls in C++ programming, and thus have more
reliable code.  But even more important: a computer program should
not only tell the machine what to do, but it should also tell /other people/
what you want the machine to do.  (For much more elaboration on this idea,
look up references on ``literate programming,'' such as [fn:Knuth84].)
This is obviously important any time
when you have many people working on a given piece of software,
and such considerations would naturally lead to code that is easy
to read and understand.  Think of writing ATLAS code as another
form of publication, and take the same care as you would writing
up an analysis for colleagues.

This document is derived from the original ATLAS C++ coding standard,
[[https://cds.cern.ch/record/685315][ATL-SOFT-2002-001]] [fn:Atlas02], which was last revised in 2003.  This itself 
derived from work done by the CERN ``Project support team''
and SPIDER project, as documented in [[http://pst.web.cern.ch/PST/HandBookWorkBook/Handbook/Programming/CodingStandard/c++standard.pdf][CERN-UCO/1999/207]] [fn:PST99].
These previous guidelines have been significantly revised
to take into account the evolution of the C++ language [fn:Cxx14],
current practices in ATLAS, and the experience gained
over the past decade.

Some additional useful information on C++ programming may be
found in [fn:Meyers97], [fn:Meyers01], and [fn:GOF].

This note is not intended to be a fixed set of rigid rules.
Rather, it should evolve as experience warrants.


* Naming

This section contains guidelines on how to name objects in a program.

** Naming of files

 - *Each class should have one header file, ending with ``.h'', and 
    one implementation file, ending with ``.cxx''.*  [<<source-naming>>]

   Some exceptions:  Small classes used as helpers for another class should
   generally not go in their own file, but should instead be placed with the
   larger class.  Sometimes several very closely related classes
   may be grouped together in a single file; in that case, the files should
   be named after whichever is the ``primary'' class.  A number of related
   small helper classes (not associated with a particular larger class)
   may be grouped together in a single file, which should be given 
   a descriptive name.  An example of the latter could be a set of classes
   used as exceptions for a package.

   For classes in a namespace, the namespace should not be included
   in the file name.  For example, the header for =Trk::Track=
   should be called =Track.h=.

   Implementation (.cxx) files that would be empty may be omitted.

   The use of the ``.h'' suffix for headers is long-standing ATLAS practice;
   however, it is unfortunate since language-sensitive editors will then default to using
   ``C'' rather than ``C++'' mode for these files.  For emacs, it can help
   to put a line like this at the start of the file:

   #+BEGIN_EXAMPLE
// This file is really -*- C++ -*-.
#+END_EXAMPLE


** Meaningful names

 - *Choose names based on pronounceable English words, common
    abbreviations, or acronyms widely used in the experiment, except*
   *for loop iteration variables.*  [<<meaningful-names>>]

   For example, =nameLength= is better than =nLn=.

   Use names that are English and self-descriptive.  Abbreviations
   and/or acronyms used should be of common use within the community.


 - *Do not create very similar names.*  [<<no-similar-names>>]

   In particular, avoid names that differ only in case.  For example,
   =track= / =Track=; =c1= / =cl=; =XO= / =X0=.


** Required naming conventions:

Generally speaking, you should try to match the conventions used by whatever
package you're working on.  But please try to always follow these rules:

 - *Use prefix* =m_= *for private/protected data members of classes.*  [<<data-member-naming>>]

   Use a lowercase letter after the prefix =m_=.

   An exception for this is xAOD data classes, where the member names are
   exposed via ROOT for analysis.


 - *Do not start any other names with* =m_=. [<<m-prefix-reserved>>]


 - *Do not start names with an underscore.  Do not use names that contain
    anywhere a double underscore.* [<<system-reserved-names>>]

   Such names are reserved for the use of the compiler and system libraries.

   The precise rule is that names that contain a double underscore or which
   start with an underscore followed by an uppercase letter are reserved anywhere,
   and all other names starting with an underscore are reserved in the global
   namespace.  However, it's good practice to just avoid all names starting
   with an underscore.


** Recommended naming conventions

If there is no already-established naming convention for the package
you're working on, the following guidelines are recommended as being
generally consistent with ATLAS usage.

 - *Use prefix* =s_= *for private/protected static data members of classes.* [<<static-members>>]

   Use a lowercase letter after the prefix =s_=.

 - *The choice of namespace names should be agreed to by the communities concerned.*  [<<namespace-naming>>]

   Don't proliferate namespaces.  If the community developing the code has
   a namespace defined already, use it rather than defining a new one.
   Examples include =Trk::= for tracking and =InDet::= for inner detector.

 - *Use namespaces to avoid name conflicts between classes.*  [<<use-namespaces>>]

   A name clash occurs when a name is defined in more than one place. For
   example, two different class libraries could give two different
   classes the same name. If you try to use many class libraries at the
   same time, there is a fair chance that you will be unable to compile
   and link the program because of name clashes. To solve the problem you
   can use a namespace.

   New code should preferably be put in a namespace, unless typical ATLAS
   usage is otherwise.  For example, ATLAS classes related to the calorimeter
   conventionally start with ``Calo'' rather than being in a C++ namespace.

 - *Start class and enum types with an uppercase letter.*  [<<class-naming>>]

   #+BEGIN_EXAMPLE
class Track;
enum State { green, yellow, red };
#+END_EXAMPLE

 - *Typedef names should start with an uppercase letter if they are public
    and treated as classes.*  [<<typedef-naming>>]

   #+BEGIN_EXAMPLE
typedef vector<MCParticleKinematics*> TrackVector;
#+END_EXAMPLE

 - *Alternatively, a typedef name may start with a lower-case letter
   and end with* =_t=.  [<<typedef-naming-2>>]

   This form should be reserved for type names which are not treated
   as classes (e.g., a name for a fundamental type) or names which
   are private to a class.

  #+BEGIN_EXAMPLE
typedef unsigned int mycounter_t;
#+END_EXAMPLE


 - *Start names of variables, members, and functions with a lowercase letter.*  [<<variable-and-function-naming>>]

   #+BEGIN_EXAMPLE
double energy;
void extrapolate();
#+END_EXAMPLE

   Names starting with =s_= and =m_= should have a lowercase letter
   following the underscore.

   Exceptions may be made for the case where the name is following
   standard physics or mathematical notation that would require
   an uppercase letter; for example, uppercase =E= for energy.

 - *In names that consist of more than one word, write the words together,
   and start each word that follows the first one with an uppercase letter.*  [<<compound-names>>]

   #+BEGIN_EXAMPLE
class OuterTrackerDigit;
double depositedEnergy;
void findTrack();
#+END_EXAMPLE

   Some ATLAS packages also use the convention that names are entirely lowercase
   and separated by underscores.  When modifying an existing package,
   you should try to be consistent with the existing naming convention.

 - *All package names in the release must be unique, independent of the
   package's location in the hierarchy.*  [<<unique-package-names>>]

   If there is a package, say ``A/B/C'', already existing, another package may
   not have the name ``D/E/C'' because that ``C'' has already been used.
   This is required for proper functioning of the build system.

 - *Underscores should be avoided in package names.*  [<<no-underscores-in-package-names>>]

   The old ATLAS rule was that the =_= should be used in package names
   when they are composites of one or more acronyms, e.g. =TRT_Tracker=,
   =AtlasDB_*=.  Underscores should be avoided unless they really help with
   readability and help in avoiding spelling mistakes. =TRTTracker= looks
   odd because of the double ``T''. Using underscores in package names will
   also add to confusion in the multiple-inclusion protection lines.

 - *Acronyms should be written as all uppercase.*  [<<uppercase-acronyms>>]

   #+BEGIN_EXAMPLE
METReconstruction, not MetReconstruction
MuonCSCValidation, not MuonCscValidation
#+END_EXAMPLE

   Unfortunately, existing code widely uses both forms.


* Coding

This section contains a set of items regarding the ``content'' of the
code. Organization of the code, control flow, object life cycle,
conversions, object-oriented programming, error handling, parts of C++
to avoid, portability, are all examples of issues that are covered
here.

The purpose of the following items is to highlight some useful ways to
exploit the features of the programming language, and to identify some
common or potential errors to avoid.


** Organizing the code

 - *Header files must begin and end with multiple-inclusion protection.* [<<header-guards>>]

   #+BEGIN_EXAMPLE
#ifndef PACKAGE_CLASS_H
#define PACKAGE_CLASS_H
// The text of the header goes in here ...
#endif // PACKAGE_CLASS_H
#+END_EXAMPLE

   Header files are often included many times in a program. Because C++
   does not allow multiple definitions of a class, it is necessary to
   prevent the compiler from reading the definitions more than once.

   The include guard should include both the package name and class name,
   to ensure that is unique.

   Don't start the include guard name with an underscore!

   In some rare cases, a file may be intended to be included multiple times,
   and thus not have an include guard.  Such files should be explicitly
   commented as such, and should usually have an extension other than ``.h''.

 - *Use forward declaration instead of including a header file,
    if this is sufficient.*  [<<forward-declarations>>]
   
   #+BEGIN_EXAMPLE
class Line;
class Point
{
public:
  // Distance from a line
  Number distance(const Line& line) const;  
};
#+END_EXAMPLE

   Here it is sufficient to say that =Line= is a class, without giving
   details which are inside its header.  This saves time in compilation
   and avoids an apparent dependency upon the =Line= header file.

   Be careful, however: this does not work if =Line= is actually a =typedef=
   (as is the case, for example, with many of the xAOD classes).

 - *Each header file must contain the declaration for one class only, except for 
   embedded or very tightly coupled classes or collections of small helper classes.*  [<<one-class-per-source>>]

   This makes your source code files easier to read. This also improves
   the version control of the files; for example the file containing a
   stable class declaration can be committed and not changed any more.

   Some exceptions:  Small classes used as helpers for another class should
   generally not go in their own file, but should instead be placed with the
   larger class.  Sometimes several very closely related classes
   may be grouped together in a single file; in that case, the files should
   be named after whichever is the ``primary'' class.  A number of related
   small helper classes (not associated with a particular larger class)
   may be grouped together in a single file, which should be given 
   a descriptive name.  An example of the latter could be a set of classes
   used as exceptions for a package.

 - *Implementation files must hold the member function definitions for
   the class(es) declared in the corresponding header file.* [<<implementation-file>>]

   This is for the same reason as for the previous item.

 - *Ordering of #include statements.*  [<<include-ordering>>]

   =#include= directives should generally be listed according to dependency
   ordering, with the files that have the most dependencies coming first.
   This implies that the first =#include= in a ``.cxx'' file should be
   the corresponding ``.h'' file, followed by other =#include= directives
   from the same package.  These would then be followed by =#include= directives
   for other packages, again ordered from most to least dependent. 
   Finally, system =#include= directives should come last.

   #+BEGIN_EXAMPLE
// Example for CaloCell.cxx
// First the corresponding header.
#include "CaloEvent/CaloCell.h"
// The headers from other ATLAS packages,
// from most to least dependent.
#include "CaloDetDescr/CaloDetDescrElement.h"
#include "SGTools/BaseInfo.h"
// Headers from external packages.
#include "CLHEP/Geometry/Vector3D.h"
#include "CLHEP/Geometry/Point3D.h"
// System headers.
#include <cmath>
#+END_EXAMPLE

   Ordering the =#include= directives in this way gives the best chance
   of catching problems where headers fail to include other headers that they
   depend on.

   Some old guides recommended testing on the C++ header guard around
   the =#include= directive.  This advice is now obsolete and should be avoided.

   #+BEGIN_EXAMPLE
// Obsolete --- don't do this anymore.
#ifndef MYPACKAGE_MYHEADER_H
# include "MyPackage/MyHeader.h"
#endif
#+END_EXAMPLE

   The rationale for this was to avoid having the preprocessor do redundant
   reads of the header file.  However, current C++ compilers do this
   optimization on their own, so this serves only to clutter the source.

 - *Do not use ``using'' directives or declarations in headers or prior to an #include.* [<<no-using-in-headers>>]

   A =using= directive or declaration imports names from one namespace 
   into another, often the global namespace.

   This does, however, lead to pollution of the global namespace.
   This can be manageable if it's for a single source file; however,
   if the directive is in a header file, it can affect many different
   source files.  In most cases, the author of these sources won't 
   be expecting this.

   Having =using= in a header can also hide errors.  For example:

   #+BEGIN_EXAMPLE
// In first header A.h:
using namespace std;

// In second header B.h:
#include "A.h"

// In source file B.cxx
#include "B.h"
...
vector<int> x;  // Missing std!
#+END_EXAMPLE

   Here, a reference to =std::vector= in B.cxx is mistakenly written without
   the =std::= qualifier.  However, it works anyway because of the
   =using= directive in A.h.  But imagine that later B.h is revised so that
   it no longer uses anything from A.h, so the =#include= of A.h is removed.
   Suddenly, the reference to =vector= in B.cxx no longer compiles.
   Now imagine there are several more layers of =#include= and
   potentially hundreds of affected source files.
   To try to prevent problems like this, headers should not use =using=
   outside of classes.  (Within a class definition, =using= can have
   a different meaning that is not covered by this rule.)

   For similar reasons, if you have a =using= directive or declaration
   in a ``.cxx'' file, it should come after all =#include= directives.
   Otherwise, the =using= may serve to hide problems with missing
   namespace qualifications in the headers.

   Starting with C++11, =using= can also be used in ways similar to =typedef=.
   Such usage is not covered by this rule.


** Control flow

 - *Do not change a loop variable inside a for loop block.*  [<<do-not-modify-for-variable>>]

   When you write a for loop, it is highly confusing and error-prone to
   change the loop variable within the loop body rather than inside the
   expression executed after each iteration.  It may also inhibit many
   of the loop optimizations that the compiler can perform.

 - *Prefer range-based for loops.*  [<<prefer-range-based-for>>]

   C++11 introduced the `range-based for'.  Prefer using this to a loop
   with explicit iterators; that is, prefer:
   #+BEGIN_EXAMPLE
std::vector<int> v = ...;
for (int x : v) {
  doSomething (x);
}
#+END_EXAMPLE

   to
   #+BEGIN_EXAMPLE
std::vector<int> v = ...;
for (std::vector<int>::const_iterator it = v.begin();
     it != v.end();
   ++it)
{
  doSomething (*it);
}
#+END_EXAMPLE

   In some cases you can't make this replacement; for example, if you
   need to call methods on the iterator itself, or you need to manage
   multiple iterators within the loop.  But most simple loops over
   STL ranges are more simply written with a range-based for.


 - *All switch statements must have a default clause.*  [<<switch-default>>]

   In some cases the default clause can never be reached because there
   are case labels for all possible enum values in the switch statement,
   but by having such an unreachable default clause you show a potential
   reader that you know what you are doing.

   You also provide for future changes. If an additional enum value is
   added, the switch statement should not just silently ignore the new
   value. Instead, it should in some way notify the programmer that the
   switch statement must be changed; for example, you could throw an
   exception


 - *Each clause of a switch statement must end with* =break=.  [<<switch-break>>]

   #+BEGIN_EXAMPLE
// somewhere specified: enum Colors { GREEN, RED }

// semaphore of type Colors

switch(semaphore) {
case GREEN:
  // statement
  break;
case RED:
  // statement
  break;
default:
  // unforeseen color; it is a bug
  // do some action to signal it
}
#+END_EXAMPLE

   If you must ``fall through'' from one switch clause to another 
   (excluding the trivial case of a clause with no statements),
   this must be explicitly stated in a comment.  This should,
   however, be a rare case.

   #+BEGIN_EXAMPLE
switch (case) {
case 1:
  doSomething();
  /* FALLTHROUGH */
case 2:
  doSomethingMore();
  break;
...
#+END_EXAMPLE

   gcc7 will warn about such constructs unless you use a comment like
   in the example above.  (C++17 will add a =fallthrough= attribute.)


 - *An if-statement which does not fit in one line must have braces
   around the conditional statement.*  [<<if-bracing>>]

   This makes code much more readable and reliable, by clearly showing
   the flow paths.

   The addition of a final else is particularly important in the case
   where you have if/else-if. To be safe, even single statements should
   be explicitly blocked by ={}=.

   #+BEGIN_EXAMPLE
if (val == thresholdMin) {
  statement;
}
else if (val == thresholdMax) {
  statement;
}
else {
  statement;    // handles all other (unforeseen) cases
}
#+END_EXAMPLE


 - *Do not use goto.*  [<<no-goto>>]

   Use =break= or =continue= instead.

   This statement remains valid also in the case of nested loops, where
   the use of control variables can easily allow to break the loop,
   without using =goto=.

   =goto= statements decrease readability and maintainability and make
   testing difficult by increasing the complexity of the code.

   If =goto= statements must be used, it's better to use them for
   forward branching than backwards, and the functions involved should
   be kept short.

** Object life cycle

*** Initialization of variables and constants

 - *Declare each variable with the smallest possible scope and initialize
    it at the same time.*  [<<variable-initialization>>]

    It is best to declare variables close to where they are
    used. Otherwise you may have trouble finding out the type of a
    particular variable.

    It is also very important to initialize the variable immediately, so
    that its value is well defined.

    #+BEGIN_EXAMPLE
int value = -1;  // initial value clearly defined
int maxValue;    // initial value undefined, NOT recommended
#+END_EXAMPLE


 - *Avoid use of ``magic literals'' in the code.*  [<<no-magic-literals>>]

    If some number or string has a particular meaning, it's best to declare
    a symbol for it, rather than using it directly.  This is especially
    true if the same value must be used consistently in multiple places.

    Bad example:

    #+BEGIN_EXAMPLE
class A
{
  ...
  TH1* m_array[10];
};

void A::foo()
{
  for (int i = 0; i < 10; i++) {
    m_array[i] = dynamic_cast<TH1*>
      (gDirectory()->Get (TString ("hist_") +
       TString::Itoa(i,10)));
}
#+END_EXAMPLE

    Better example:

    #+BEGIN_EXAMPLE
class A
{
  ...

  static const s_numberOfHistograms = 10;
  static TString s_histPrefix;
  TH1* m_array[s_numberOfHistograms];
};

TString s_histPrefix = "hist_";

void A::foo()
{
  for (int i = 0; i < s_numberOfHistograms; i++) {
    TString istr = TString::Itoa (i, 10); // base 10
    m_array[i] = dynamic_cast<TH1*>
     (gDirectory()->Get (s_histPrefix + istr);
}
#+END_EXAMPLE

    It is not necessary to turn /every/ literal into a symbol.
    For example, the `10' in the example above in the =Itoa= call,
    which gives the base for the conversion, would probably not benefit
    from being made a symbol, though a comment might be helpful.
    Similarly, sometimes =reserve()= is called on a =std::vector= before
    it is filled with a value that is essentially arbitrary.  It probably
    also doesn't help to make this a symbol, but again, a comment would
    be helpful.  Strings containing text to be written as part of a log message
    are also best written literally.

    In general, though, if you write a literal value other than `0', `1',
    =true=, =false=, or a string used in a log message, you should consider
    defining a symbol for it.


 - *Declare each type of variable in a separate declaration statement, and
   do not declare different types (e.g. int and int pointer) in one declaration statement.*  [<<separate-declarations>>]

    Declaring multiple variables on the same line is not recommended. The
    code will be difficult to read and understand.

    Some common mistakes are also avoided. Remember that when you declare
    a pointer, a unary pointer is bound only to the variable that
    immediately follows.

    #+BEGIN_EXAMPLE
int i, *ip, ia[100], (*ifp)();   // Not recommended

// recommended way:
LoadModule* oldLm = 0;  // pointer to the old object
LoadModule* newLm = 0;  // pointer to the new object
#+END_EXAMPLE

    Bad example: both =ip= and =jp= were intended to be pointers to integers,
    but only =ip= is --- =jp= is just an integer!

    #+BEGIN_EXAMPLE
int* ip, jp;
#+END_EXAMPLE



 - *Do not use the same variable name in outer and inner scope.*  [<<no-variable-shadowing>>]

    Otherwise the code would be very hard to understand; and it would
    certainly be very error prone.

    Some compilers will warn about this.


 - *Be conservative in using* =auto=.  [<<using-auto>>]

   C++11 includes the new =auto= keyword, which allows one to omit explicitly
   writing types that the compile can deduce.  Examples:

   #+BEGIN_EXAMPLE
auto x = 10;  // Type int deduced
auto y = 42ul;  // Type unsigned long deduced.
auto it = vec.begin();  // Iterator type deduced
#+END_EXAMPLE

   Some authorities have recommended using =auto= pretty much everywhere 
   you can (calling it ``auto almost always'').  However, our experience
   has been that this adversely affects the readability and robustness
   of the code.  It generally helps a reader to understand what the code
   is doing if the type is apparent, but with =auto=, it often isn't.
   Using =auto= also makes it more difficult to find places where 
   a particular type is used when searching the code with tools
   like lxr.  It can also make it more difficult to track errors back
   to their source:

   #+BEGIN_EXAMPLE
const Foo* doSomething();
...  a lot of code here ...
auto foo = doSomething();
// What is the type of foo here?  You have to look up
// doSomething() in order to find out!  Makes it much
// harder to find all places  where the type Foo gets used.

// If the return type of doSomething() changes, you'll get
// an error here, not at the doSomething() call.
foo->doSomethingElse();
#+END_EXAMPLE

   The current recommendation is to generally not use =auto= in place of a
   (possibly-qualified) simple type:

   #+BEGIN_EXAMPLE
// Use these
int x = 42;
const Foo* foo = doSomething();
for (const CaloCell* cell : caloCellContainer) ...
Foo foo (x);

// Rather than these
auto x = 42;
auto foo = doSomething();
for (auto cell : caloCellContainer) ...
auto foo = Foo {x};
#+END_EXAMPLE

   There are three sorts of places where it generally makes sense to
   use =auto=.

     - When the type is already evident in the expression and the declaration
       would be redundant.  This is usually the case for expressions
       with =new= or =make_unique=.

       #+BEGIN_EXAMPLE
// auto is fine here.
auto foo = new Foo;
auto ufoo = std::make_unique<Foo>();
#+END_EXAMPLE

     - When you need a declaration for a complicated derived type, where
       the type itself isn't of much interest.

       #+BEGIN_EXAMPLE
// Fine to use auto here; the full name of the type
// is too cumbersome to be useful.
std::map<int, std::string> m  = ..;
auto ret = m.insert (std::make_pair (1, "x"));
if (ret.second) ....
#+END_EXAMPLE

     - =auto= may also be useful in writing generic template code.
   
   In general, the decision as to whether or not to use =auto= should
   be made on the basis of what makes the code easier to _read_.
   It is bad practice to use it simply to save a few characters
   of typing.


*** Constructor initializer lists

 - *Let the order in the initializer list be the same as the order of the
    declarations in the header file: first base classes, then data members.*  [<<member-initializer-ordering>>]

    It is legal in C++ to list initializers in any order you wish, but you
    should list them in the same order as they will be called.

    The order in the initializer list is irrelevant to the execution order
    of the initializers. Putting initializers for data members and base
    classes in any order other than their actual initialization order is
    therefore highly confusing and can lead to errors.

    Class members are initialized in the order of their declaration in the
    class; the order in which they are listed in a member initialization
    list makes no difference whatsoever! So if you hope to understand what
    is really going on when your objects are being initialized, list the
    members in the initialization list in the order in which those members
    are declared in the class.

    Here, in the bad example, =m_data= is initialized first (as it appears
    in the class) /before/ =m_size=, even though =m_size= is listed first.
    Thus, the =m_data= initialization will read uninitialized data from =m_size=.

    Bad example:

    #+BEGIN_EXAMPLE
class Array
{
public:
  Array(int lower, int upper);
private:
  int* m_data;
  unsigned m_size;
  int m_lowerBound;
  int m_upperBound;
};
Array::Array(int lower, int upper) :
  m_size(upper-lower+1),
  m_lowerBound(lower),
  m_upperBound(upper),
  m_data(new int[m_size])
{ ...
#+END_EXAMPLE

    Correct example:

    #+BEGIN_EXAMPLE
class Array
{
public:
  Array(int lower, int upper);
private:
  unsigned m_size;
  int m_lowerBound;
  int m_upperBound;
  int* m_data;
};
Array::Array(int lower, int upper) :
  m_size(upper-lower+1),
  m_lowerBound(lower),
  m_upperBound(upper),
  m_data(new int[m_size]) { ...
#+END_EXAMPLE

    Virtual base classes are always initialized first, then base classes,
    data members, and finally the constructor body for the derived class
    is run.

    #+BEGIN_EXAMPLE
class Derived : public Base   // Base is number 1
{ 
public:
  explicit Derived(int i);
  // The keyword explicit prevents the constructor
  // from being called implicitly.
  //   int x = 1;
  //   Derived dNew = x;
  // will not work

  Derived();

private:
  int m_jM;   // m_jM is number 2
  Base m_bM;  // m_bM is number 3
};

Derived::Derived(int i) : Base(i), m_jM(i), m_bM(i) {
// Recommended order        1       2        3
...
}
#+END_EXAMPLE


*** Copying of objects

 - *A function must never return, or in any other way give access to,
    references or pointers to local variables outside the scope in which they are declared.*  [<<no-refs-to-locals>>]

    Returning a pointer or reference to a local variable is always wrong
    because it gives the user a pointer or reference to an object that no
    longer exists.

    Bad example:

    You are using a complex number class, =Complex=, and you write a method
    that looks like this:

    #+BEGIN_EXAMPLE
Complex&
calculateC1 (const Complex& n1, const Complex& n2)
{
  double a = n1.getReal()-2*n2.getReal();
  double b = n1.getImaginary()*n2.getImaginary();

  // create local object
  Complex C1(a,b);

  // return reference to local object
  // the object is destroyed on exit from this function:
  // trouble ahead!
  return C1;
}
#+END_EXAMPLE

    In fact, most compilers will spot this and issue a warning.

    This particular function would be better written to return the result
    by value:

    #+BEGIN_EXAMPLE
Complex calculateC1 (const Complex& n1, const Complex& n2)
{
  double a = n1.getReal()-2*n2.getReal();
  double b = n1.getImaginary()*n2.getImaginary();

  return Complex(a,b);
}
#+END_EXAMPLE


 - *If objects of a class should never be copied, then the copy constructor
    and the copy assignment operator should be deleted.*  [<<copy-protection>>]

    Ideally the question whether the class has a reasonable copy semantic
    will naturally be a result of the design process. Do not define a copy
    method for a class that should not have it.

    By deleting the copy constructor and copy assignment operator, you can make a
    class non-copyable.

    #+BEGIN_EXAMPLE
// There is only one ATLASExperimentalHall,
// and that should not be copied
class ATLASExperimentalHall
{
public:
  ATLASExperimentalHall();
  ~ATLASExperimentalHall();

  // Delete copy constructor --- disallow copying.
  ATLASExperimentalHall(const ATLASExperimentalHall& )
     = delete;

  // Delete assignment operator --- disallow assignment.
  ATLASExperimentalHall&
  operator=(const ATLAS_ExperimentalHall&) = delete;
};
#+END_EXAMPLE

    This syntax was new in C++11.  In C++98, this was achieved
    by declaring the deleted methods as private (and not implementing them).

    #+BEGIN_EXAMPLE
// There is only one ATLASExperimentalHall,
// and that should not be copied
class ATLASExperimentalHall
{
public:
  ATLASExperimentalHall();
  ~ATLASExperimentalHall();

private:
  // Disallow copy constructor and assignment.
  ATLASExperimentalHall(const ATLASExperimentalHall& );
  ATLASExperimentalHall& operator=
    (const ATLAS_ExperimentalHall&);
};
#+END_EXAMPLE

 - *If a class owns memory via a pointer data member, then the copy constructor,
   the assignment operator, and the destructor should all be implemented.*  [<<define-copy-and-assignment>>]

    The compiler will generate a copy constructor, an assignment
    operator, and a destructor if these member functions have not been
    declared. A compiler-generated copy constructor does memberwise
    initialization and a compiler-generated copy assignment operator does
    memberwise assignment of data members and base classes. For classes
    that manage resources (examples: memory (new), files, sockets) the
    generated member functions probably have the wrong behavior and must
    be implemented by the developer. You have to decide if the resources
    pointed to must be copied as well (deep copy), and implement the
    correct behavior in the operators. Of course, the constructor and
    destructor must be implemented as well.

    Bad Example:

    #+BEGIN_EXAMPLE
class String
{
public:
  String(const char *value=0);
  ~String(); // destructor but no copy constructor
             // or assignment operator
private:
  char *m_data;
};

String::String(const char *value)
{ // correct behavior implemented in constructor
  m_data = new char[strlen(value)]; // fill m_data
}
String::~String()
{ // correct behavior implemented in destructor
  delete m_data;
}

...

// declare and construct a ==> m_data points to "Hello"
String a("Hello");

// open new scope
{ // declare and construct b ==> m_data points to "World"
  String b("World");

  b=a;
  // execute default op= as synthesized by compiler ==>
  // memberwise assignment i.e. for pointers (m_data)
  //  bitwise copy
  // ==> m_data of "a" and "b" now point to the same string
  //     "Hello"
  // ==> 1) memory b used to point to never deleted ==>
  //        possible memory leak
  //     2) when either a or b goes out of scope,
  //        its destructor  will delete the memory
  //        still pointed to by the other
}

// close scope: `b''s destructor called;
// memory still pointed to by `a' deleted!
String c=a;
// but m_data of "a" is undefined!!
#+END_EXAMPLE

 - *Assignment member functions must work correctly when the left and right operands are the
    same object.* [<<self-assign>>]

    This requires some care when writing assignment code, as the case
    (when left and right operands are the same) may require that most of
    the code is bypassed.

    #+BEGIN_EXAMPLE
A& A::operator=(const A& a)
{
  if (this != &a) {
    // beware of s=s - "this" and "a" are the same object
    // ... implementation of operator=
  }
}
#+END_EXAMPLE

** Conversions

 - *Use explicit rather than implicit type conversion.*  [<<avoid-implicit-conversions>>]

   Most conversions are bad in some way. They can make the code less
   portable, less robust, and less readable. It is therefore important to
   use only explicit conversions. Implicit conversions are almost always
   bad.

   In general, casts should be strongly discouraged, especially the old
   style C casts.


 - *Use the C++ cast operators* (=dynamic_cast= *and* =static_cast=) *instead
    of the C-style casts.*  [<<use-c++-casts>>]

   The new cast operators give the user a way to distinguish between
   different types of casts, and to ensure that casts only do what is intended
   and nothing else.

   The C++ =static_cast= operator allows explicitly requesting allowed
   implicit conversions and between integers and enums.  It also allows 
   casting pointers up and down a class hierarchy (as long as there's no
   virtual inheritance), but no checking is done when casting from 
   a less- to a more-derived type.

   The C++ =dynamic_cast= operator is used to perform safe casts down or
   across an inheritance hierarchy. One can actually determine whether
   the cast succeeded because failed casts are indicated either by a
   =bad_cast= exception or a null pointer. The use of this type of
   information at run time is called Run-Time Type Identification (RTTI).

   #+BEGIN_EXAMPLE
int n = 3;
double r = static_cast<double>(n) * a;

class Base { };
class Derived : Base { };
void f(Derived* d_ptr)
{
  // if the following cast is inappropriate
  //  a null pointer will be returned!
  Base* b_ptr = dynamic_cast<Base*>(d_ptr);
  // ...
}
#+END_EXAMPLE


 - *Do not convert const objects to non-const.*  [<<no-const-cast>>]

   In general you should never cast away the constness of objects.

   If you have to use a =const_cast= to remove =const=, either you're writing
   some low-level code that that's deliberately subverting the C++ type system,
   or you have some problem in your design or implementation that the
   =const_cast= is papering over.

   Sometimes you're forced to use a =const_cast= due to problems with external
   libraries.  But if the library in question is maintained by ATLAS,
   then try to get it fixed in the original library before resorting
   to =const_cast=.

   The keyword =mutable= allows data members of an object that have been
   declared const to remain modifiable, thus reducing the need to cast
   away constness. The =mutable= keyword should only be used for variables which
   are used for caching information. In other words, the object appears
   not to have changed but it has stored something to save time on
   subsequent use.

 - *Do not use* =reinterpret_cast=. [<<no-reinterpret-cast>>]

   =reinterpret_cast= is machine-, compiler- and
   compile-options-dependent. It is a way of forcing a compiler to accept
   a type conversion which is dependent on implementation. It blows away
   type-safety, violates encapsulation and more importantly, can lead to
   unpredictable results.

   =reinterpret_cast= has legitimate uses, such as low-level code which deliberately
   goes around the C++ type system.  Such code should usually be found only
   in the core and framework packages.

   Exception: =reinterpret_cast= is required in some cases if one is not
   using old-style casts. It is required for example if you wish to
   convert a callback function signature (X11, expat, Unix signal
   handlers are common causes). Some external libraries (X11 in
   particular) depend on casting function pointers. If you absolutely
   have to work around limitations in external libraries, you may of
   course use it.

   One particularly nasty case to be aware of and to avoid is /pointer aliasing/.
   If two pointers have different types, the compiler may assume that they
   cannot point at the same object.  For example, in this function:

   #+BEGIN_EXAMPLE
int convertAndBuffer (int* buf, float x)
{
  float* fbuf = reinterpret_cast<float*>(buf);
  *fbuf = x;
  return *buf;
}
#+END_EXAMPLE

   the compiler is entitled to rewrite it as

   #+BEGIN_EXAMPLE
int convertAndBuffer (int* buf, float x)
{
  int ret = *buf;
  float* fbuf = reinterpret_cast<float*>(buf);
  *fbuf = x;
  return ret;
}
#+END_EXAMPLE

   (As a special case, you can safely convert any pointer type to or from
   a =char*=.)  The proper way to do such a conversion is with a union.


** The class interface

*** Inline functions

 - *Header files must contain no implementation except for small functions 
    to be inlined. These inlines must appear at the end of the header after*
   *the class definition.* [<<inline-functions>>]

    If you have many inline functions, it is usually better to split them
    out into a separate file, with extension ``.icc'', that is included
    at the end of the header.

    Inline functions can improve the performance of your program; but they
    also can increase the overall size of the program and thus, in some cases,
    have the opposite result. It can be hard to know exactly when inlining
    is appropriate.  As a rule of thumb, inline only very simple functions
    to start with (one or two lines).  You can use profiling information
    to identify other functions that would benefit from inlining.

    Use of inlining makes debugging hard and, even worse, can force a
    complete release rebuild or large scale recompilation if the inline definition
    needs to be changed.



*** Argument passing and return values

 -  *Pass an unmodifiable argument by value only if it is of built-in type or small; otherwise, pass the argument by const reference (or by const pointer
     if it may be null).*  [<<large-argument-passing>>]

    An object is considered small if it is a built-in type or if it
    contains at most one small object.  Built-in types such as =char=, =int=,
    and =double= can be passed by value because it is cheap to copy such variables. If
    an object is larger than the size of its reference (typically 64
    bits), it is not efficient to pass it by value. Of course, a built-in
    type can be passed by reference when appropriate.

    #+BEGIN_EXAMPLE
void func(char c);   // OK
void func(int i);    // OK
void func(double d); // OK
void func(complex<float> c); // OK

void func(Track t); // not good, since Track is large, so
                    // there is an overhead in copying t.
#+END_EXAMPLE

    Arguments of class type are often costly to copy, so it is
    preferable to pass a =const= reference to such objects; in this way the argument
    is not copied. Const access guarantees that the function will not
    change the argument.

    In terms of efficiency, passing by pointer is the same as passing by reference.
    However, passing by reference is preferred, unless it is possible to the
    object to be missing from the call.

    #+BEGIN_EXAMPLE
void func(const LongString& s);  // const reference
#+END_EXAMPLE


 -  *If an argument may be modified, pass it by non-const reference
    and clearly document the intent.*  [<<modifiable-arguments>>]

    For example:

    #+BEGIN_EXAMPLE
// Track @c t is updated by the addition of hit @c h.
void updateTrack(const Hit& h, Track& t);
#+END_EXAMPLE

    Again, passing by references is preferred, but a pointer may be used
    if the object can be null.

 -  *Use* =unique_ptr= *to pass ownership of an object to a function.*
    [<<pass-ownership>>]

    To pass ownership of an object into a function, use =unique_ptr= 
    (by value):

    #+BEGIN_EXAMPLE
void foo (std::unique_ptr<Object> obj);

...

  auto obj = std::make_unique<Object>();
  ...
  foo (std::move (obj);
#+END_EXAMPLE

    In most cases, =unique_ptr= should be passed by _value_.  There are
    however a few possible use cases for passing =unique_ptr= by reference:

     - The called function may replace the object passed in with another one.
       In this case, however, consider returning the new object as the
       value of the function.

     - The called function may only conditionally take ownership of the
       passed object.  This is likely to be confusing and error-prone
       and should probably be avoided.  Consider if a =shared_ptr=
       would be better in this case.

    There is basically no good case for passing =unique_ptr= as a const reference.

    If you need to interoperate with existing code, object ownership
    may be passed by pointer.
    The fact that ownership is transferred should be clearly documented.

    _Do not_ pass ownership using references.

    Here are a some additional examples to illustrate this.
    Assume that class =C= contains a member =Foo* m_owning_pointer=
    which the class deletes.  (In C++11, it would of course usually be better for
    this to be a =unique_ptr=.)
    #+BEGIN_EXAMPLE
   // --- Best 
   void C::takesOwnership (std::unique_ptr<Foo> foo)
   {
      delete m_owning_pointer;
      m_owning_pointer = foo.release();
   }

   // --- Ok if documented.
   // Takes ownership of the @c foo pointer.
   void C::takesOwnership (Foo* foo)
   {
      delete m_owning_pointer;
      m_owning_pointer = foo;
   }

   // --- Don't do this!
   void C::takesOwnership (Foo& foo)
   {
      delete m_owning_pointer;
      m_owning_pointer = &foo;
   }
#+END_EXAMPLE

 - *Return basic types or new instances of a class type by value*. [<<return-by-value>>]

    Returning a class instance by value is generally preferred to passing
    an argument by non-const reference:

    #+BEGIN_EXAMPLE
// Bad
void getVector (std::vector<int>& v)
{
  v.clear();
  for (int i=0; i < 10; i++) v.push_back(v);
}

// Better
std::vector<int> getVector()
{
  std::vector<int> v;
  for (int i=0; i < 10; i++) v.push_back(v);
  return v;
}
#+END_EXAMPLE

    The return-value optimization plus move semantics will generally mean
    that there won't be a significant efficiency difference between the two.

 - *Use* =unique_ptr= *to return ownership.* [<<returning-ownership>>]

    If a function is returning a pointer to something that is allocated off the heap
    which the caller is responsible for deleting, then return a =unique_ptr=.

    If compatibility with existing code is an issue,
    then a plain pointer may be used, but the caller takes ownership should
    be clearly documented.

    _Do not_ return ownership via a reference.

    #+BEGIN_EXAMPLE
// Best
std::unique_ptr<Foo> makeFoo()
{
  return std::make_unique<Foo> (...);
}

// Ok if documented
// makeFoo() returns a newly-allocated Foo; caller must delete it.
Foo* makeFoo()
{
  return new Foo (...);
}

// NO!
Foo& makeFoo()
{
  Foo* foo = new Foo (...);
  return *foo;
}
#+END_EXAMPLE


 - *Have* =operator== *return a reference to* =*this=.  [<<assignment-return-value>>]

    This ensures that

    #+BEGIN_EXAMPLE
a = b = c;
#+END_EXAMPLE

    will assign =c= to =b= and then =b= to =a= as is the case with built-in objects.


*** =const= correctness


 - *Declare a pointer or reference argument, passed to a function, as const if the function does not
    change the object bound to it.*  [<<const-arguments>>]

    An advantage of =const=-declared parameters is that the compiler will
    actually give you an error if you modify such a parameter by mistake,
    thus helping you to avoid bugs in the implementation.

    #+BEGIN_EXAMPLE
// operator<< does not modify the String parameter
ostream& operator<<(ostream& out, const String& s);
#+END_EXAMPLE


 - *The argument to a copy constructor and to an assignment operator
   must be a const reference.* [<<copy-ctor-arg>>]

    This ensures that the object being copied is not altered by the copy or assign.


 - *In a class method, do not return pointers or non-const references to private data members.*
   [<<no-non-const-refs-returned>>]

    Otherwise you break the principle of encapsulation.

    If necessary, you can return a pointer to a =const= or =const= reference.

    This does not mean that you cannot have methods returning an =iterator=
    if your class acts as a container.

    An allowed exception to this rule if the use of the singleton
    pattern.  In that case,
    be sure to add a clear explanation in a comment so
    that other developers will understand what you are doing.


 - *Declare as const a member function that does not affect the state of the object.* [<<const-members>>]

    Declaring a member function as =const= has two important implications.
    First, only =const= member functions can be called for =const= objects;
    and second, a =const= member function will not change data members

    It is a common mistake to forget to =const= declare member functions
    that should be =const=.

    This rule does not apply to the case where a member function which
    does not affect the state of the object overrides a non-const member
    function inherited from some super class.


 - *Do not let const member functions change the state of the program.* [<<really-const>>]

    A =const= member function promises not to change any of the data members
    of the object.  Usually this is not enough. It should be possible to
    call a =const= member function any number of times without affecting the
    state of the complete program. It is therefore important that a =const=
    member function refrains from changing static data members or other
    objects to which the object has a pointer or reference.

*** Overloading and default arguments

 - *Use function overloading only when methods differ in their argument list,
    but the task performed is the same.*

    Using function name overloading for any other purpose than to group
    closely related member functions is very confusing and is not
    recommended.

** =new= and =delete=


 - *Do not use new and delete where automatic allocation will work.* 
   [<<auto-allocation-not-new-delete>>]

   Suppose you have a function that takes as an argument a pointer to an object,
   but the function does not take ownership of the object.  Then suppose you
   need to create a temporary object to pass to this function.  In this case,
   it's better to create an automatically-allocated object on the stack
   than it is to use =new= / =delete=.  The former will be faster, and you
   won't have the chance to make a mistake by omitting the =delete=.

   #+BEGIN_EXAMPLE
  // Not good:
  Foo* foo = new Foo;
  doSomethingWithFoo (foo);
  delete foo;

  // Better:
  Foo foo;
  doSomethingWithFoo (&foo);
#+END_EXAMPLE



 - *Match every invocation of new with one invocation of delete in all
   possible control flows from new.* [<<match-new-delete>>]

   A missing delete would cause a memory leak.

   However, in the Gaudi/Athena framework, an object created with
   =new= and registered in StoreGate is under
   control of StoreGate and must not be deleted.

   In new code, you should generally use =make_unique= for this.

   #+BEGIN_EXAMPLE
#include <memory>

  ...
  DataVector<C>* dv = ...;
  auto c = std::make_unique<C>("argument");
  ...
  if (test) {
    dv->push_back (std::move (c));
  }
#+END_EXAMPLE

   =auto_ptr= was an attempt to do something similar to =unique_ptr= in older
   versions of the language.  However, it has some serious deficiencies
   and should not be used in new code.


 - *A function should explicitly document if it takes ownership of a
   pointer passed to it as an argument.*  [<<explicit-ownership>>]