Coding Conventions and Developer Guide

Coding Conventions

Naming Conventions

The object member names are lowercase, start with 'm_' and each word is split with an underscore (m_class_member)
The static object member names are lowercase, start with 's_' and each word is split with an underscore (s_static_member)
The object names are lowercase and each word is split with an underscore (class_name)
The template arguments have the first letter of each word capitalized, no underscores (TemplateArgument)
The local variables are lowercase and the words are separated by underscores (same as for object names, well, it's not easy mistake them)
The typedef names are lowercase, words separated by underscores, and they end with _t (some_def_t)
The class methods and all the functions follow the same convention: lowercase, words separated by underscores
TODO Getter & Setters?
TODO class names?
enum types follow the convention of the object names, but are defined inside the enumtype namespace (enumtype::some_id)

Namespaces:

Q: What conventions do we use? we have gridtools, and some _impl. Any type that is internal under _impl, the rest under gridtools?

Formatting

Always use spaces, never tabs.
Indent with exactly 4 spaces.
Try to keep line width no longer than 120 characters. Code in an IDE with large screens looks good with longer lines, but code review tools like github does not play well with long lines.

C++

Scoping

Never use using keyword from header files.
Do not use nested classes, except for metafunctions
Place a function's variables in the narrowest scope possible, and initialize at the declaration.

//not ok
int a,b,c,d; //only definition, not initialization
b = 10; 
d = 10;
for(a=0; a<b; ++a)
{
    c=3; //define c in the scope where you use it
}

int b=10; 
int d=10;
for(int a=0; a<b; ++a)
{
    int c=3;
}

Inheritance

Make sure inheritance models a "is a" relationship, otherwise use composition.

//not ok 
class Bus : public Ship 
...
//ok
class Bus: public Vehicle
...

Never redefine an inherited non virtual method

class Base
{
    void foo() { printf("base\n");}
}
class Subclass : public Base
{
    void foo() { printf("sub\n"}; //redefinition of foo
}
Subclass* sub = new Subclass();
sub->foo(); //prints sub
static_cast<Base*>(sub)->foo(); //prints base

PAOLO: OK as general rule, but not too strict: I find it useful in certain occasions to override behaviours in the subclasses, see the arg_decorator implementation

Classes

You must define a default constructor if your class defines member variables and has no other constructors. Otherwise the compiler will do it for you, badly. For example

class Rectangle
{
public:
    void Rectangle()
    {
        // initialize variables with impossible default value
        // -> debugging easier
        m_width = -1;
        m_heigth = -1;
    }
private:
    int m_width;
    int m_height;
};

Do never rely on default initialization of compiler
Always disable the copy constructor and assignment operators of a class (unless you explicitly provide those) by means of the DISALLOW_COPY_AND_ASSIGN macro.

class Foo 
{
    DISALLOW_COPY_AND_ASSIGN(Foo);
public: 
    Foo(int f);
    ~Foo();

If a class has virtual methods, the destructor must be virtual
Pass primitive types (int, double, etc.) by value, unless it is an output parameter of the function.
Other input only arguments are passed by const reference, except the argument can be NULL, then a const pointer is used.

Pointers

Use references whenever possible (instead of pointers), i.e. when assignment happens at initialization time and the reference does not change over time.
Always assert pointer before using them

assert(ptr);
ptr->xx();

Try to use RAII, i.e. use smart pointers for non performance critical sections:

boost::shared_ptr<MyClass> ptr = boost::make_shared<MyClass>();

Use delete[] for freeing array of pointers.

metaprograms

Every type passed as a template argument is checked (asserted). This increase both safety and readability (from trait that checks the type we infer its function)

template<
    typename MssArray, 
    typename Coords, 
    typename MssLocalDomainArray, 
    enumtype::backend BackendId, 
    enumtype::strategy StrategyId
>
struct mss_functor
{
    BOOST_STATIC_ASSERT((
        is_sequence_of<
            MssLocalDomainArray, 
            is_mss_local_domain
        >::value));
    BOOST_STATIC_ASSERT((
        is_meta_array_of<MssArray, is_mss_descriptor>::value
    ));
    BOOST_STATIC_ASSERT((is_coordinates<Coords>::value));
    ....
}

Avoid "Blob" anti-pattern (see Chapter 2.2 of Book C++ Template Metaprogramming from David Abrahams), where one metafunction computes multiple output. It decreases readability. For example in the following example, run_functor_traits is a blob anti-pattern

template<typename run_functor_derived>
class run_functor
{
   typedef typename run_functor_traits<run_functor_derived>::
       execute_policy execute_policy_t;
   typedef typename run_functor_traits<run_functor_derived>::
       arguments arguments_t;
   typedef typename run_functor_traits<run_functor_derived>::
       local_domain local_domain_t;
}

Replace it by single output type patterns:

template<typename run_functor_derived>
class run_functor
{
   typedef typename run_functor_arguments<run_functor_derived>::type 
      arguments_t;
   typedef typename run_functor_local_domain<run_functor_derived>::type 
      local_domain_t;
}

Developer's Guide

Code Structure

Different classes with different functionality go into different header files. Do not build .h files with a blob of classes.
Name of header files is after name of the class. For example local_domain.h contains the local_domain class
Put metafunctions associated to, or act on a type in a <type>_metafunctions.h file. For example the metafunction

template<...> struct make_interval{...};

will be placed in interval_metafunctions.h

Tests

Everything is tested: runtime functions, metdhos, functors but as well metafunctions. If you write new classes or method, provide the corresponding metafunctions before creating a pull request.
Write all tests of a class or functionality from <type>.h into a file in the unit_tests folder, named test_<type>.h

Documentation

Use doxygen documentations. Below is an example

    /**
     * @brief functor that executes all the functors contained within the mss
     * @tparam MssArray meta array containing all the mss descriptors
     * @tparam Coords coordinates
     * @tparam MssLocalDomainArray sequence of mss local domain (each contains the local domain list of a mss)
     * @tparam BackendId id of backend
     * @tparam StrategyId id of strategy
     */
    template<typename MssArray, typename Coords, typename MssLocalDomainArray, enumtype::backend BackendId, enumtype::strategy StrategyId>
    struct mss_functor
    {

        /**
         * @brief This is a do something method
         * @param local_domain_lists this is a param
         * @param coords this is a param
         * @param block_idx this is a param
         * @param block_idy this is a param
         * @return this is the return value
         */
        static int DoSomething(MssLocalDomainArray& local_domain_lists, const Coords& coords, const int block_idx, const int block_idy) :
             {...}

Git & Code Review

Create one branch per functionality. Avoid personal branches that mix multiple functionality and keep growing.
Always merge and close a branch after a pull request is completed (do not continue developing in the branch)
Do not create branches from branches (unless you a have a good reason), always branch from master

Before merging into master

Any branch candidate for merge into the trunk, for which we have created a pull request should have been compiled and tested for all the different cmake options (at least the ones that generate different compilations), like GPU=ON, OFF, CXX11=ON|OFF, DEBUG|Release, etc.
master should always be in a healthy state. It must not contain bug nor performance bugs. That means it compiles, and run for all backends and cmake options, and performance numbers for all backends are matching the reference values.
Check the performance for all backends for the representative benchmarks. Verify that performance was not degraded with respect to reference.