Skip to content

GitLab

Commit a5a2e30d authored by scott snyder's avatar scott snyder
Browse files

Add discussions of pointer aliasing, argument passing, and auto. Formatting fixes.

parent 371effd5
Hide whitespace changes
Inline Side-by-side
rules.org
View file @ a5a2e30d
......@@ -75,28 +75,28 @@ This section contains guidelines on how to name objects in a program.
- *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
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
......@@ -107,16 +107,16 @@ to put a line like this at the start of the file:
abbreviations, or acronyms widely used in the experiment, except*
*for loop iteration variables.* [<<meaningful-names>>]
For example, =nameLength= is better than =nLn=.
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.
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=.
In particular, avoid names that differ only in case. For example,
=track= / =Track=; =c1= / =cl=; =XO= / =X0=.
** Required naming conventions:
......@@ -126,10 +126,10 @@ 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_=.
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.
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>>]
......@@ -138,13 +138,13 @@ exposed via ROOT for analysis.
- *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.
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.
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
......@@ -155,30 +155,30 @@ 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_=.
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.
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.
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.
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
#+BEGIN_EXAMPLE
class Track;
enum State { green, yellow, red };
#+END_EXAMPLE
......@@ -186,73 +186,73 @@ enum State { green, yellow, red };
- *Typedef names should start with an uppercase letter if they are public
and treated as classes.* [<<typedef-naming>>]
#+BEGIN_EXAMPLE
#+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.
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
#+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
#+BEGIN_EXAMPLE
double energy;
void extrapolate();
#+END_EXAMPLE
Names starting with =s_= and =m_= should have a lowercase letter
following the underscore.
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.
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
#+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.
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.
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.
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
#+BEGIN_EXAMPLE
METReconstruction, not MetReconstruction
MuonCSCValidation, not MuonCscValidation
#+END_EXAMPLE
Unfortunately, existing code widely uses both forms.
Unfortunately, existing code widely uses both forms.
* Coding
......@@ -272,31 +272,30 @@ common or potential errors to avoid.
- *Header files must begin and end with multiple-inclusion protection.* [<<header-guards>>]
#+BEGIN_EXAMPLE
#+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.
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.
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!
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''.
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
#+BEGIN_EXAMPLE
class Line;
class Point
{
......@@ -306,47 +305,46 @@ public:
};
#+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).
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.
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.
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.
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.
=#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
#+BEGIN_EXAMPLE
// Example for CaloCell.cxx
// First the corresponding header.
#include "CaloEvent/CaloCell.h"
......@@ -361,40 +359,38 @@ Finally, system =#include= directives should come last.
#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.
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.
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
#+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.
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.
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 managable 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.
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:
Having =using= in a header can also hide errors. For example:
#+BEGIN_EXAMPLE
#+BEGIN_EXAMPLE
// In first header A.h:
using namespace std;
......@@ -407,50 +403,48 @@ using namespace std;
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.
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.
** 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.
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.
- *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.
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
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
#+BEGIN_EXAMPLE
// somewhere specified: enum Colors { GREEN, RED }
// semaphore of type Colors
......@@ -468,24 +462,23 @@ default:
}
#+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.
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.
- *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.
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 ={}=.
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
#+BEGIN_EXAMPLE
if (val == thresholdMin) {
statement;
}
......@@ -500,18 +493,18 @@ else {
- *Do not use goto.* [<<no-goto>>]
Use =break= or =continue= instead.
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=.
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.
=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.
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
......@@ -520,14 +513,14 @@ be kept short.
- *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 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.
It is also very important to initialize the variable immediately, so
that its value is well defined.
#+BEGIN_EXAMPLE
#+BEGIN_EXAMPLE
int value = -1; // initial value clearly defined
int maxValue; // initial value undefined, NOT recommended
#+END_EXAMPLE
......@@ -535,13 +528,13 @@ int maxValue; // initial value undefined, NOT recommended
- *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.
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:
Bad example:
#+BEGIN_EXAMPLE
#+BEGIN_EXAMPLE
class A
{
...
......@@ -557,10 +550,9 @@ void A::foo()
}
#+END_EXAMPLE
Better example:
Better example:
#+BEGIN_EXAMPLE
#+BEGIN_EXAMPLE
class A
{
...
......@@ -581,32 +573,32 @@ void A::foo()
}
#+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