#+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''.* [<>] 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.* [<>] 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.* [<>] 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.* [<>] 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_=. [<>] - *Do not start names with an underscore. Do not use names that contain anywhere a double underscore.* [<>] 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.* [<>] Use a lowercase letter after the prefix =s_=. - *The choice of namespace names should be agreed to by the communities concerned.* [<>] 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.* [<>] 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.* [<>] #+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.* [<>] #+BEGIN_EXAMPLE typedef vector TrackVector; #+END_EXAMPLE - *Alternatively, a typedef name may start with a lower-case letter and end with* =_t=. [<>] 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.* [<>] #+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.* [<>] #+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.* [<>] 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.* [<>] 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.* [<>] #+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.* [<>] #+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.* [<>] #+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.* [<>] 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.* [<>] This is for the same reason as for the previous item. - *Ordering of #include statements.* [<>] =#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 #+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.* [<>] 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 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.* [<>] 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.* [<>] C++11 introduced the `range-based for'. Prefer using this to a loop with explicit iterators; that is, prefer: #+BEGIN_EXAMPLE std::vector v = ...; for (int x : v) { doSomething (x); } #+END_EXAMPLE to #+BEGIN_EXAMPLE std::vector v = ...; for (std::vector::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.* [<>] 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=. [<>] #+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.* [<>] 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.* [<>] 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.* [<>] 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.* [<>] 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 (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 (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.* [<>] 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.* [<>] 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=. [<>] 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(); #+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 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.* [<>] 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.* [<>] 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.* [<>] 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.* [<>] 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.* [<>] 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.* [<>] 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.* [<>] 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(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(d_ptr); // ... } #+END_EXAMPLE - *Do not convert const objects to non-const.* [<>] 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=. [<>] =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(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(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.* [<>] 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).* [<>] 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 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.* [<>] 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.* [<>] To pass ownership of an object into a function, use =unique_ptr= (by value): #+BEGIN_EXAMPLE void foo (std::unique_ptr