Coding Rules

  • NB: last update for OpenViBE 2.0.0. OpenViBE 1.3.0 compliant version

Introduction

New coding rules have been provided and followed during the CertiViBE project. They should be followed for any contribution to OpenViBE. However, if somebody is modifying existing code, the recommended practice is to follow the convention already present in the file or change the whole convention of the file, but not introduce mixed convention.

Naming Rules

Language

US English must be used to name any entities (file, class, variable etc.) in the system.

Explicit Naming

The name should provide enough information to understand the purpose of the named entity.
Ex: InputDataBuffer is more explicit that Buffer
Ex: ComputePerimeter is more explicit than Compute

File header

The whole source code of the project won’t have the same license. So two headers are available regarding the license needed. The choice of the license for a file is made regarding the intellectual property define in CertiViBE contract.

Public header

File header for code under AGPL license.

/*********************************************************************
 * Software License Agreement (AGPL-3 License)
 *
 * CertiViBE
 * Based on OpenViBE V1.1.0, Copyright (C) Inria, 2006-2015
 * Copyright (C) Inria, 2015-2017,V1.0
 *
 * Author
 *
 * This program is free software: you can redistribute it and/or modify
 * it under the terms of the GNU Affero General Public License version 3,
 * as published by the Free Software Foundation.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 * GNU Affero General Public License for more details.
 *
 * You should have received a copy of the GNU Affero General Public License
 * along with this program.
 * If not, see <http://www.gnu.org/licenses/>.
 */

Private header

File header for private code under Mensia property.

/*********************************************************************
** INRIA and MENSIA TECHNOLOGIES SA, CONFIDENTIAL                    *
**                                                                   *
** CertiViBE Test Software                                           *
** Copyright Inria and Mensia Technologies SA, 2015-2017             *
** All Rights Reserved                                               *
**                                                                   *
** Author                                                            *
**                                                                   *
** NOTICE: All information contained herein is, and remains          *
** the property of Mensia Technologies SA and Inria.                 *
** The intellectual and technical concepts contained                 *
** herein are proprietary to Mensia Technologies SA and Inria,       *
** and are covered copyright law.                                    *
** Dissemination of this information or reproduction of this material*
** is strictly forbidden unless prior written permission is obtained *
** from Mensia Technologies SA and Inria.                            *
*********************************************************************/

C/C++ Coding Rules

Naming

Entity Prefix Naming Template
Class C CMyClass
Struct S SMyStruct
Template class T TMyTemplateClass
Interface (abstract class) I IMyInterface
Enum E EMyEnum
Function/method No prefix myFunction
Class/struct private member variable m_ m_MyVariable
Class/Struct public member variable No prefix myClassVariable
Local variable No prefix myLocalVariable
Function/Method parameter No Prefix myParameter

Formatting

Place braces “{}” which enclose a block in the same column

// RIGHT
if ( i == 5 )
{
}
// WRONG
if ( i == 5 ) {
}


Follow flow control primitives if, else, while, for and do by a block

// RIGHT
if ( i == 5 )
{
    return -1 ;
}
// WRONG
if ( i == 5 ) return -1 ;


Group ‘*’ and ‘&’ with the type, not the variable

// RIGHT
float* a

// WRONG
float *a


Do not use spaces around ‘.’ or ‘->’, nor between unary operators and operands

// RIGHT
a->b
!isValid()

// WRONG
a-> b
! isValid()


Use parentheses to clarify the order of evaluation for operators in expressions

// RIGHT
( a * b ) + ( c * d )

// WRONG
a * b + c * d


Beginning of line indentation is made with tabs, not spaces, but in-line indentation is made with spaces, not tabs

// RIGHT
for(int i = 1; i < 2; i++) 
{
--->printf(“foobar %s\n”,.token);
--->printf(“foo %s\n”,....token);
--->printf(“bar %s\n”,....token);
}

// WRONG
for(int i = 1; i < 2; i++) 
{
....printf(“foobar %s\n”,.token);
....printf(“foo %s\n”,> token);
....printf(“bar %s\n”,....token);
}


Indent #if statements
By default, the visual studio editor left flushes all preprocessing macros. This makes the code really hard to read, especially when the macros are nested. As a general rule, configure your editor to not automatically modify code in a style not conform with this document. Instead, indent your macros just as you would normal C code:

// RIGHT
void f()
{
  #ifdef _WIN32
    #define RUNNINGS_WINDOWS
    #ifdef PRODUCTION
      bool print_error_messages = true
    #else
      bool print_error_messages = false
    #endif
  #else
    bool win32 = false
  #endif
}

// WRONG
void f()
{
#ifdef _WIN32
#define RUNNINGS_WINDOWS
#ifdef PRODUCTION
  bool print_error_messages = true
#else
  bool print_error_messages = false
#endif
#else
  bool win32 = false
#endif
}

File System

  • Class/Function declaration should reside in a header file (.h)
  • Class/Function definition should reside in an implementation file (.cpp)

Exception: Inline implementation files should have .inl extension

  • Private header files (used in templates) should have .hpp extension
  • All files must be encoded in UTF-8
  • All files in source control should have UNIX (LF) line endings.
    • Note for Git: on Windows always use “core.autocrlf = true”

Never edit files on Windows partitions from UNIX systems (e.g.: mounted drive on a virtual machine)

Includes

      • Header decoration
        • Former header file should be decorated by #ifndef, #define, #endif directives to avoid recursive inclusion of headers
        • Newly created file shoud use #pragma once directive
      • When using the standard libc, include C++ headers instead of C headers

// RIGHT
#include <cstring>
// WRONG
#include <cstring.h>

    • Don’t use #include if a forward declaration would suffice
    • Don’t use platform-specific inclusion in header files
    • Header files must contain all their dependencies (or forward declaration to them)
    • Header files should not contain useless dependencies. Try to avoid using generic include (xxx_all.h) that will slow down the compilation.

API

  • Do not expose third party objects in public interfaces including STL objects
  • Do not expose functions that could potentially throw exceptions
  • Limit the public API to pure virtual interfaces


Please read this article to fully understand the requirements behind designing binary compatible interfaces.

Namespace

  • Protect libraries with a namespace
  • Do not use using namespace directive in header file

Class and Struct

  • Use a struct for passive data carrier

Apart from contructors and destructors, a struct should not have any method that change the state of the object (i.e only const methods).

  • Struct data members are public. Do not add private members to Structs (by default all members will be public)
  • Class data members are private (not public neither protected)
  • Provide a copy constructor and assignment operator only when necessary. Otherwise, disable them with the = delete syntax available in C+11.

If the class is part of a hierarchy, copy and assignment should be disabled. If copy is necessary, prefer a specific clone method (rule 54 of C++ Coding Standards: 101 Rules, Guidelines, and Best Practices).
It the class is a value class, copy and assignment should be enabled.


Refer to C++ 99 rules of 3 or C++ 11 rules of 5 for further information.


class NonCopyable
{
    protected:
        NonCopyable() = default;
        ~NonCopyable() = default;
    private:
        NonCopyable(const NonCopyable&) = delete;
        NonCopyable& operator=(const NonCopyable&) = delete;
}

  • Define a virtual destructor for all classes which are used as base classes and which have virtual function
  • Declare classes that should not be extended as final.

Most classes that are highly specific should be final.

Functions

  • Function parameter order: output, update, input

Place output parameters on the left, followed by update parameters (i.e. those that are input as well as output), and put input parameters at the end.

void doFoo(char* output, const char* input0, int input1, int input2);

  • Avoid the use of numeric values in code; use symbolic values instead

No 3.14159265359 in the code, use M_PI. If no symbolic value exists for your magic number define it.
Prefer to define constants as static const variables in an anonymous namespace rather than #define macros.

  • Use numeric_limits features to assign boundary values to variable

#include 

void function()
{
    // RIGHT
    unsigned int upperLimit = std::numeric_limits::max();

    // WRONG
    unsigned int upperLimit = (unsigned int)-1;
}

  • Consider using an STL container to return multiple items


This is only true for private API features (cf. API Rules)

  • Accessors should start with “set” or “get”


Exceptions: if an accessor reads a boolean value, it may begin with “is” or “has”

class MyClass
{
...
    void setSpeed(float speed);
    float getSpeed()const;
    void setActive(bool active);
    bool isActive() const;
    bool hasSpeedControl() const;
...
}

    • Naming semantic

The following function name pairs must be used in order to keep consistency within the framework.

      • create/release

Of an object, where a C++ constructor is not appropriate. This is the case if an object can be re-created multiple times. Use this instead of placement new if possible. It makes the code clearer.

      • initialize / uninitialize

Used for systems and managers.

      • start / stop

start() is used to start something running, either synchronously or asynchronously, until stop() is called. For example: a particle effect, a music track, a streaming system, or a timer.
add / remove
An element in a collection.

      • open / close

Of a data stream. Input/output, or between processes.

Const correctness

    • Make class member method const unless there is a reason to do otherwise
    • Pass input parameters by const reference


Exception: fundamental type can be passed by value (i.e. int, float, char)

Coding Good Practice

  • Use C++ casts like static_cast<>()

// RIGHT
int data = static_cast(short_data);

// WRONG
int data = (int)short_data;

  • Use C++ 11 range based loop to perform repetitive action on each element of a container

Indexed loops should be kept for specific purposes where the index is necessary.

  • Use C++ 11 “auto” as a facilitator


“auto” aims at making things easier to write/read. However, it is sometimes better to use the explicit type when this one is not obvious.

  • Use C++ 11 “std::nullptr” as pointer literal

// RIGHT
char *pointer = std::nullptr;

// WRONG
char *pointer = NULL;
char *pointer = 0;

  • Use C++ 11 smart pointer in implementations to ensure RAII

// RIGHT
bool MyClass::myFunction()
{
    std::unique_ptr resource1(new Resource());
    ...
    if(failed)
    {
       return false
    }
    ...
    return true;
}

// WRONG
bool MyClass::myFunction()
{
    Resource* resource1 = new Resource();
    ...
    if(failed)
    {
       delete resource1;
       return false
    }
    ...
    delete resource1;
    return true;
}

  • Use lambda functions to simulate RAII when there are more than 1 return path

Most of the framework APIs are not RAII-proof. Thus cleanup of the resource must be performed explicitly.

// RIGHT
bool MyClass::myFunction()
{
    std::unique_ptr resource1(new Resource1());
    std::unique_ptr resource2(new Resource2());
    resource1->open();
    resource2->initialize();

    auto cleanup = [&]() {
    resource2->uninitialize();
    resource1->close();
    };
    ...
    if(failed)
    {
        cleanup();
        return false;
    }
    ...
    cleanup();
    return true;
}

// WRONG
bool MyClass::myFunction()
{
    std::unique_ptr resource1(new Resource1());
    std::unique_ptr resource2(new Resource2());
    resource1->open();
    resource2->initialize();
    ...
    if(failed)
    {
        resource2->uninitialize();
        resource1->close();
        return false;
    }
    ...
    resource2->uninitialize();
    resource1->close();
    return true;
}

  • Use multiple inheritance only if necessary

Sometimes, it can make code much easier to maintain and read than with using framework legacy template inheritance.


Warning: If you do so, beware the dread diamond problem!

Compilations/Linking/Static Analysis

  • Compile without compiler warnings
  • Static analysis with cppcheck must return no Error and no Warning

Comments and Documentation

  • Use Doxygen syntax for interface documentation

Use Javadoc-syntax for documentation (/// and /**) and put documentation in the header files.

  • The documentation should cover the whole API

Every function / class in the public AND private API should be commented with doxygen.

  • Use // for descriptive comments

// comments are better for comments that you want to leave in the code, because they don’t have any nesting problems, it is easy to see what is commented, etc.

  • Use comments as hints to the reader

Add comment to explain the global goal of a code section. The code should be self-explanatory on the “how” it achieve this goal:

// BAD
/// Returns the speed of the vehicle
float sp() {return _sp;}
// Computes speed from distance and time
s = d / t;
// Check for end of file
if (c == -1)


Instead, write code that is self-explanatory.

float speed() {return _speed;}
speed = distance / time;
if (c == END_OF_FILE_MARKER)


Source code comments should be used as hints to the reader who tries to understand the code. They should point out when the code does something which is a little bit clever or tricky, something that may not be immediately obvious from reading just the code. In complicated algorithms that consist of several steps, they are also useful for identifying the separate steps and giving the user a sense of context.

// Use Duff's device for loop unrolling
// See for example: http://en.wikipedia.org/wiki/Duff's_device
switch (count % 8)
{
    case 0: do { *to = *from++;
    case 7: *to = *from++;
    case 6: *to = *from++;
    case 5: *to = *from++;
    case 4: *to = *from++;
    case 3: *to = *from++;
    case 2: *to = *from++;
    case 1: *to = *from++;
} while ((count -= 8) > 0);
}
//Note: Do not use Duff's device anywhere

Dependencies

  • Use already included dependencies when possible

We already rely on several third party technologies that cover our needs so far. If a need is not covered by the dependencies we already considered, it might be that the feature is not common and it is interesting to evaluate the cost of using a new dependency vs creating our own implementation of the feature.
Such choice has direct impact on the IEC 62304 compliance and the SOUP management; refer to relevant documents for more details. Thus including a new dependency must be discussed.

  • Do not create strong relation between our code and weak dependencies

Libraries that compile on a limited number of platforms are known to have doubtful interactions with other libraries or are suspected to have limited evolution in the future should be isolated in separate modules.
As example, we could cite the following OpenViBE modules:

  • The EBML module implements the EBML spec in order to leave out the libebml reference implementation
  • The XML module has been implemented to hide the expat library which could in turn be replaced later on with a very small impact on the OpenViBE software
  • The FS module gathers a few features inspired from boost filesystem handling but do not rely on boost itself, avoiding the boost dependency

Python Coding Rules

General Rules

Any python scripts should follow the PEP-8 style guide.


List of recommended tools: PyCharm (IDE), pep8 (Linux pep8 validator), pylint (linux python linter).

CMake Coding Rules

General Rules

Some people tends to consider cmake scripts as “not as important” as c/c++ source files. It leads to a cmake scripts having a much lower coding standard than the rest of the project.
It should not be the case.

  • Use lower case for cmake commands in new files (in legacy files, either stick to the file command case or change every commands to lower case)
  • Use upper case for cmake variables with underscore as separator

// RIGHT
SET MY_VARIABLE
// WRONG
set MyVariable

  • Use new macro for repetitive tasks
  • Split your files in specific sections (configure, build, test etc)
  • Split a file in many subfiles if a file becomes too large
  • Add comment when the goal of section is not obvious
This entry was posted in Architecture and practices. Bookmark the permalink.