-
Notifications
You must be signed in to change notification settings - Fork 17
Coding conventions
In general, we have adopted many of the same standards as the ROOT project along with some conventions from the C++ Core Guidlines.
- Naming conventions
- Header files
- Object Ownership
- Adding a class
- Adding a library
- Formatting code for PRs
Each folder in the root directory names AtName
will build into a shared object with the name libAtName
as defined by a CMakelists.txt
file in each folder. Other folders, (eg macro, icon, parameters, etc.), are not built and do not contain a CMakeLists.txt folder. In order for the new library to be built, it must be added as a subdirectory to the global CMakeList.txt in the root directory.
Classes that do not live in a namespace should have the prefix "At", followed by a descriptive name in camel case. For example, the class AtBaseEvent
lives in the AtData
library with source and header files AtData/AtBaseEvent.cxx
and AtData/AtBaseEvent.h
, respectively.
Following the ROOT naming conventions, new classes and methods should avoid back-to-back capital letter. For example the method name for getting the x-axis from a ROOT histogram is GetXaxis
, not GetXAxis
.
All data members of a class should be private or protected and begin with the letter f
, followed by a capital letter. Boolean flags should begin with the letter k
. The exception to this is if the class is a glorified struct where there is no data verification happening and setters/getters add no behavior just unnecessary code. All member functions should begin with a capital letter.
All source files should have the extension .cxx
and all headers should have the extension .h
.
Namespaces do not follow any particular naming convention except the should start with a capital letter and are camel case.
Tests should live alongside the code they are testing to simplify navigating back and forth between production code and test code.
Tests should be named UnitNameTest.cxx
. For example the AtBaseEvent
(with associated files AtData/AtBaseEvent.h
and AtData/AtBaseEvent.cxx
) tests would be located in AtData/AtBaseEventTest.cxx
. Appending the suffix Test
keeps the tests close to the code when the files are listed alphabetically.
Any fakes should be named FakeClassFaked.cxx
. By appending the prefix Fake
we keep all fakes close together in the file explorer.
Test fixtures that belong to a base class and are shared by the derived types should be kept in their own header files. Fixtures should follow the naming convention ClassTestFixture
, for example AtPSATestFixture
that lives in the file AtPSATestFixture.h
Never include a header file when a forward declaration is sufficient. Only include header files for base classes or classes that are used by value in the class definition in a class header. Instead, you should put the majority of your #include
statements in the source file with the class implementation.
Following ROOT's style guide, private data members should be declared first, followed by the private static members, the private methods and the private static methods. Then the protected members and methods, and finally the public methods. Add trivial get or setters directly in the class definition. Add more complex inlines (longer than one line) at the bottom of the .h file.
Header files referencing this code base should be included using quotes i.e. #include "header.h"
. Header files from dependencies should be included using angle brackets i.e. #include <Rtypes.h>
. This is the syntax assumed by the custom mapping file we use for running IWYU and will lead to false errors running the linter if deviated from.
Every header file requires an associated .cxx
file in the same directory, even if it is an empty file, except for a single #include "fileName.h"
. This is required by the build system.
When we talk about object ownership we mean the right to free the associated resources (in otherwords delete
an object). ROOT has some weird quirks of ownership, where you may not own the things you think you do. This is particularly true of histograms, TTrees, and TEventLists. Details of ROOT ownership practices can be found here. For everything not discussed here we use the following conventions (modified from the C++ Core Guidlines:
- Express ownership explicitly through use of smart pointers
std::unique_ptr
andstd::shared_ptr
. This means there should never be an explicit call tonew
ordelete
.- The exception to this is ROOT cannot yet create streamer for writing
std::shared_ptr
, orstd::unique_ptr
with a custom Deleter (iestd::unqiue_ptr<class T, class Deleter>
withclass Deleter != std::default_delete<T>
.std::unqiue_ptr<class T>
are allowed and required) to disk. In that case, a raw pointer (T*
) with a comment explaining the intended ownership is acceptable until ROOT fixes that problem.
- The exception to this is ROOT cannot yet create streamer for writing
- Prefer
std::unique_ptr
overstd::shared_ptr
unless ownership must be shared. - Raw pointers (
T*
) and raw references are non-owning (T&
). They should never be deleted. - Take smart pointers as parameters only to express lifetime semantics (transfer ownership)
- In interfaces (headers), use raw pointers to denote individual objects. Arrays should be represented by a container (unless it is c-style string).
Classes can be added by creating the header and source files. Classes should respect the naming convention of AtName.cxx
for source, and AtName.h
for headers or the build system might fail. Classes should only inherit from TObject
if it is required (i.e. it needs to be written to disk, or for RTTI. When in doubt do not extend TObject
. Likewise, classes should ONLY use the ClassDef()
and ClassImp()
macros if the class inherits from TObject. In the library directory the CMakeLists.txt
file must be edited to add any include directories needed as well as add the source file so CMake knows to compile it. In addition the class should be added the the local LinkDef.h
file with the generation of streamers and operator<<
disabled. For example the entry in the LinkDef.h
for the class AtName would look like: #pragma link C++ class AtName - ! ;
. This insures the library containing the class can still be properly loaded by the ROOT interpreter at runtime (ie the class ends up in the rootmap file).
An object should always be created in a valid state. Instance variables that are set to the same value in all constructors should be instantiated in the header file. Variables whose initial value can change depending on the constructor called, should be initialized in the constructor's braced-init-list. Prefer delegating constructors to repeating code.
In general, any class added or modified should respect backwards compatibility. Breaking of the public interface should only be done with good reason and in collaboration with the current maintainer(s). In addition, it should avoid modifying important base classes unless there is a good reason. That is, if you're adding a feature only used in a certain experiment to a certain class you should extend that class rather then modify it.
When a class needs to support IO, then it must inherit from TObject
and make use of the ClassDef()
and ClassImp()
macros. Avoid raw c types for anything that might be written to disk (any member variable). Instead use ROOT defined types like Int_t
defined in <Rtypes.h>
. If any changes are made to the memory layout of a class, the version number in the ROOT macro ClassDef
needs to be incremented. If the class overrides any virtual function, the macro ClassDefOverride
should be used instead.
The class needs to be added to the local LinkDef.h
file with the generation of streamers and operator<<
enabled. For example the entry in the LinkDef.h
for the class AtName would look like: #pragma link C++ class AtName + ;
.
Each task class (something that inherits from FairTask) should be primarily responsible for setting up the input and output. It is also a class that needs to support IO, so follow the conventions in Adding a class with IO. The logic of the task should be handled by another class (which likely does not support IO, and therefor does not need to inherit from TObject
). An instance of this "logic" class should be a member of the task class. Because the logic class does not implement IO, make sure you disable the writing of this member variable to disk. This is done with a comment //!
after the member variable. If you also like to use a Doxygen comment to describe the member you can use the comment style ///<!
. The goal here is to separate the IO from the actual logic of the task. This makes it easier for someone linking against ATTPCROOT to use the logic without spinning up an entire instance of FairRoot.
The process for detailing submitting a pull request is located in the git workflow.
Before submitting a pull request, reformat the code using clang-format
. At the time of writing, the code is formatted using clang-format
version 16. If run from within the repository it will pick up the .clang-format
file detailing the style. This file is also reproduced on the page detailing the ROOT coding style. If for some reason you need a section of code to not be formatted, you can turn off formatting using comment flags. Formatting of code is turned off with the comment // clang-format off
and re-enabled with the comment // clang-format on
. This process can be simplified using the command git-clang-format
which when given no options will turn clang-format
on all lines of code that differ between the working directory and HEAD (all files that are stashed). Detailed documentation is here.
If code has already been committed, then git-clang-format
will not know to change it. To format an individual file, you can run clang-format -i fileName
. The -i
flag tells the program to modify the file directly rather than sending the modified file to stdout. clang-format -i --verbose fileName
will print out the files modified if you are running the command on a directory, for example clang-format -i --verbose *.cxx
. You can reformat every header and source file in the repousing the script $VMCWORKDIR/scripts/formatAll.sh