Skip to content

Coding Guidelines

Jump to bottom
Bernhard Froehler edited this page Jul 10, 2024 · 17 revisions

See also the .clang-format file embedded in the source code; this codifies some of the rules laid out here; note that the .clang-format file still probably needs some refinement to fully fit our expectations, and therefore does not reflect exactly the style that is currently applied to the source files.

Code Format

  • Preprocessor defines should be all upper case, e.g. MY_PREPROCESSOR_DEFINE. Avoid them though if possible (and use constants/methods/... instead).
  • All names use CamelCase/camelCase (instead of e.g. underscore; i.e. camelCase, not: camel_case)
  • Constant names start with an upper case letter, e.g. MyConstantValue
  • Variable names start with a lower case letter, e.g. myVariableValue
  • Method names start with a lower case letter, e.g. myMethod
  • Classes in open_iA need to start with the prefix "iA".
  • Member variables should be prefixed with m_, unless they are part of a data structure with exclusively data fields (no methods), then they don't need a prefix.
  • Getter/Setter methods are written in the Qt style: setFoo(FooType newFoo) / FooType foo().
  • Place the opening brace on the line after the thing you're starting; example:
class iATest
{
    // ...
}

void foo(bool bar)
{
}
  • Always use braces, even if only one statement is to be executed in a branch; i.e. Do:
    if (bar)
    {
        oneStatement();
    }

Not:

    if (bar)
        oneStatement();

Includes

There should be as few includes as possible in a header file (ideally none). You only need to include a header file here if you:

  • Derive from a class/struct
  • Use a class/struct as member (but not if you are only using a pointer, reference or smart pointer to that class)

In all other cases (e.g. when declaring a pointer to a class member), forward-declare any referenced classes.

Example header file:

 #include "BaseClass.h"
 #include "Member.h"
 #include "MySmartPointer.h"
 
 // forward-declarations
 class Type1;
 class Type2;
 class Type3;
 class Type4;
 
 class TestIncludes: public BaseClass // BaseClass.h needs to be included
 {
     Type1* a; // fwd-declaration of PointerType is enough!
     MySmartPointer<Type2> b; // MySmartPointer needs to be included, but Type2 only fwd-declared!
     Member c; // directly using class Member as member, so we also need to include
     void testMethod(Type3 foo, Type4 & bar); // Type3 and Type4 only need to be fwd-declared
 };

In the cpp file, includes should be sorted from application-specific to the most generic headers (typically STL), in order of dependency (http://stackoverflow.com/questions/2762568/c-c-include-file-order-best-practices). The ideal order for our application code therefore is:

  • header file for this .cpp file
  • header files from the same module
  • header files from the core
  • library headers:
    • itk
    • vtk
    • Qt
    • STL & system/platform-specific includes The .clang-format file will enforce this kind of ordering.

C++ Standard

open_iA requires a compiler capable of at least C++ 17 support. Especially recommended is the usage of the override keyword to mark methods that override a virtual method from a base class, as this can help prevent subtle bugs which otherwise might be quite hard to find.

Include Guards / Pragma Once

Since all supported compilers (Visual C++, g++) support it, the simpler #pragma once should be used.

Avoid using namespace std

using namespace std, while sometimes making it a bit easier to write names from the std namespace (e.g. std::vector or std::string) should be avoided, as it pulls in all sorts of functions and might lead to weird compilation errors, e.g. on other compilers or when new standards are introduced), or even weird bugs. See e.g. this stackoverflow question for more details.

Precompiled headers

Enable by marking openiA_PRECOMPILE in CMake. This will enable precompiled headers (supported with CMake 3.16). However, in our experiments, it only slightly decreased the build time, but significantly increased the binary directory size.

Use QString

Whenever possible, use the QString type within open_iA for strings. Only when calling external library code that requires std::string or char *, convert to these types. This avoids encoding errors, as QString works with an UTF-8 encoded string; std::string and char * both are encoding-agnostic, and if not carefully handled, this can lead to problems with special characters. For conversion of a QString to a std::string or char * used in a system library for accessing files, use the method getLocalEncodingFileName from libs/base/iAFileUtils.h.

Smart Pointers

Use smart pointers instead of raw pointers whenever possible. Prefer std::shared_ptr over QSharedPointer and std::unique_ptr over QScopedPointer whenever possible. Use std::make_shared / std::make_unique instead of std::shared_ptr(new ...) / std::unique_ptr(new ...).

Dataset types

Rule of Thumb: Use generic iADataSet whenever possible. Only when you need a specific dataset type (image/mesh/...) convert to the respective required dataset type, and retrieve the required ITK/VTK image or mesh dataset from it. Use helper methods in iAITKIO.h and iATypedCallHelper.h to make filters applicable for multiple different image types.

Qt signals/slots

Use the new mechanism for specifying signal/slot connections:

    connect(foo, &iAMySignalSource::mysignal, bar &iAMySlotClass::myslot);

Not:

    connect(foo, SIGNAL(mysignal(int)), bar, SLOT(myslot(int)));

Rationale: With the new mechanism, the compiler can already check whether the connection "works" (e.g. whether the given methods exist and whether their arguments have matching types). With the old method, this would only turn up during runtime (you'd get a message in the "Output" window which is easily lost).

For overloaded signals, slots, there is also a solution; most commonly encountered will be e.g. to handle the change of a QSpinBox::valueChanged signal:

    connect(spinBox, QOverload<int>::of(&QSpinBox::valueChanged), myclass, &iAMyClass::someSlot);

instead of

    connect(spinBox, SIGNAL(valueChanged(int)), myclass, SLOT(someSlot(int));

For more information, see:

Code Smells to avoid

Things to look out for in the code; recommendations what to do when encountering them, as well as justifications for why.

  • Code duplication
    • Recommendation: merge implementations!
  • Multiple constants for the same thing
    • Recommendation: merge to use the same definition!
  • Unused ...
    • local variables, members, method parameters (also if they are assigned to, but never read)
    • methods (i.e. not called from anywhere)
    • Don't keep them just because "they might be useful in the future". They make the code unnecessarily more effort to read
    • Recommendation: remove!
  • Variables declared very early / far away from where they are used
    • Without initialization or with meaningless, empty initialization
    • Passing references to methods, which are never used in the place calling the method
      • Intended as optimization when the method is called multiple times
      • "optimization" of not declaring variable multiple times in method not worth the added unreadability!
    • Recommendation:
  • Methods that have one or more boolean parameter that switches between large parts of execution flows in the methods
    • Recommendation: See if they can be refactored, e.g. split up into multiple methods

open_iA Documentation, licensed under CC BY-NC-SA 4.0

Clone this wiki locally