Code Guidelines

This page holds all the coding guidelines that we use to develop OpenDungeons. They probably won't differ very much from other guidelines and predefined IDE editor settings, so if you have some coding experiences in one or more of the most common IDEs you should get used very quickly to the OpenDungeons source code style.

Most likely the guidelines won't ever be complete (especially the optimization part). New code and new developers sometimes means new situations that the guidelines don't cover. In such a case we'll find a consensus as soon as possible and adjust the guidelines.

While coding, if you discover code that does not follow these guidelines, you are encouraged to change it accordingly.

General information

We use C++. Any C-style logic should be avoided and all C++ advantages should be used.

Here are some examples:

// GOOD, C++
//Except constand, avoid creating global variables and singleton classes.
#include <iostream>

static_cast<int>(someOtherType);

// BAD, C
#include <cstdio>
#include <stdio.h>

(int)someOtherType;

The project makes use of the new features in the C++11 standard, however some C++11 features should not be used due to a lack of support in some compilers:

• Thread library (Use SFML or boost instead)
• String conversion functions (std::to_string, std::stof etc..)

Every code file should have a doxygen compatible comment at the beginning holding some basic information about the file, followed by the copyright statement.

/*!
 *  Copyright (C) 2011-2015  OpenDungeons Team
 *
 *  This program is free software: you can redistribute it and/or modify
 *  it under the terms of the GNU General Public License as published by
 *  the Free Software Foundation, either version 3 of the License, or
 *  (at your option) any later version.
 *
 *  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 General Public License for more details.
 *
 *  You should have received a copy of the GNU General Public License
 *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */

Don't include everything only because you could need it in the far future, instead only include what you really need now. In header files use forward declarations, if possible. Order the includes by this pattern:

// this file's header
#include "ThisFile.h"

// project headers
#include "MyFileA.h"
#include "MyFileB.h"

// external headers
#include <Ogre.h>
#include <CEGUI.h>

// C++ standard headers
#include <iostream>

Style

This part covers code style related aspects.

Indentation

The indentation of code relative to its parent should always be four spaces. Make sure to not use tabulators.

They are interpreted differently in different text programs: if your IDE is set to Tab = 4 spaces, the IDE of someone else could be set to e.g. 2 or 8 spaces.

bool function()
{
    return true;
}

Spacing

Spaces around operators and after inline semicolons and commas, No spaces around brackets of any kind.

int x = 0;

for(int i = 1; i < 5; ++i)
{
    x = 4 + i;
    someFunction(x, i);
}

Pointers

The pointer operators * and & go directly after the type, without spaces.

Type* somePointer; // right

Type *somePointer; // wrong

End of Line

We use Unix-style line endings (LF, "\n").

If you use Windows make sure the line endings won't be messed up when you push a commit. The .gitattributes file should take care of that when using git.

Line length

The code logic should make long lines unnecessary in the first place. If it's inevitable then try to make a line break around 80 characters (including indentation). But it's OK if you sometimes exceed it by some characters (no need to break line because of two characters).

The new line should always be after associated code and always before operators (because the operators are associated with their following operands). The new lines are indented relatively to their parent line with either single or double indentations, depending on the type of expression: Single indentation on simple expressions and double indent on loops and similar types of code that have their own indented code block. Reason: emphasize the difference between the long line and the wrapped code.

aVeryLongFunctionNameWithALotOfParameters(int parameterA, int parameterB,
    int parameterC); // single indent

if(parameterA == 4 || parameterB == 4 || parameterC == 4 || parameterD == 4
        || parameterE == 4) // double indent
{    
    // code
}

Trailing spaces

Try to avoid empty lines that contain spaces, or unnecessary spaces at the end of code lines (trailing spaces). Modern IDEs set indentations automatically but some don't remove them if the line isn't used at all.

/* This example uses the underscore _ to demonstrate the spaces
   (they wouldn't be visible if we took real spaces). The IDE
   automatically inserted the indent, but an empty line
   should be empty. */

void someFunction()
{
    // the next line is BAD due to trailing spaces at the end of a statement.
    int foo;__
    // the next line is BAD due to trailing spaces in an empty line.
____
    int bar;
}

unsigned vs. unsigned int

Use the full type ''unsigned int''. While both do exactly the same, there are also other unsigned types like ''unsigned char'' that explicitly need their base type. So out of consistency and clarity the full type should be used.

unsigned int foo; // GOOD
unsigned bar;     // BAD

int vs int32_t

Also, size based integers types should be used, especially for outer API, such as return values for methods and functions. You can keep on using int for trivial loops (even if size_t is maybe a better thing) but otherwise using e.g.: uint32_t, int32_t is strongly encouraged, as it permits to ensure more architecture/platform agnostic code.

Naming, comments and documentation

In general the coders should try to make the code very readable and understandable. There are two ways of achieving this:

• Give good, intuitive and auto-explaining names to functions, variables, ...
• Further explanations go into doxygen compatible comments before the code that needs explanation.

Documentation is done in the headers, not in the code.

To make it easier to distinguish between different types of code (variable, function, class, ...) the different code parts have different naming schemes which are described in the following.

// BAD, what's this function doing?
std::string up(std::string s)
{
    // code
}

// GOOD, self-explaining names and doxygen comment
//! \brief Convert a string to uppercase letters
std::string convertToUppercase(std::string strToConvert)
{
    // code
}

File names

Files are named like the class they contain.

Examples: ''MyCoolClass.cpp'', ''MyCoolClass.h''

Class names

CamelCase. First letter uppercase, every new word begins uppercase. The Name should contain nouns (because classes are used to create ''objects'').

Examples: ''MyCoolClass''

Function names

camelCase. First letter lowercase, every new word begins uppercase. The name should contain a verb and optionally a noun (functions ''do'' something to something)

Examples: ''doSomeStuff()'', ''createCreature()''

Variable names

camelCase. First letter lowercase, every new word begins uppercase.

Examples: ''speed'', ''sourceLocation'', ''goldDigged''

Private variable names

mCamelCase. First letter is m, every new word begins uppercase.

Examples: ''mSpeed'', ''mSourceLocation'', ''mGoldDigged''

Comments

For single line comments use the double-slash, for multi-line comments use the slash-asterisk notation. If possible or appropriate make the comments doxygen compatible.

/** this is a GOOD multi-line comment,
the following three code lines do nothing,
they are useless */
int i = 0;
++i;
--i;

int i; // GOOD, double-slash on single line

// Other kind of
// multiline comment

//! Multiline comment
//! for functions descriptions.

int j; /* BAD, don't use slash-asterisk on single lines */

Optimization

This part covers code optimization related aspects. A good general collection of optimizations including graphs of time measurements can be found [here](http://www.tantalon.com/pete/cppopt/main.htm|C++ Optimization Strategies and Techniques) (it goes far beyond the needs of the OpenDungeons code).

A helpful tool is Cppcheck. It analyzes source code for possible bugs and optimizations, like unused variables, scope optimizations, const correctness, obsolete functions, post/prefix increments, and so on. Once in a while we let it analyze the code and fix the stuff it finds. But if you follow these guidelines while coding it won't find much.

When and where to declare variables or include?

• If you need it now and here!
• Not if you only think you'll need it later!

This has the simple reason that you will most likely forget about it and declare/include something else (either because you simply didn't remember or because you had a better idea that doesn't need your declaration/inclusion anymore) - and then you have dead code laying around.

Variables should be declared as close as possible to where they are used. This makes it clear what the variable is used for, and when it's used.

Try to put as few includes into headers as possible. If it's needed only in the cpp file put it in the cpp file, not in the corresponding header. Use forward declarations where possible. Don't include just-in-case, as every included header adds to the build time.

Prefix increment

In most cases prefix in- and decrementing should happen. Especially in loops. This has the simple reason that with postfix the CPU needs to copy and store the old value while it doesn't need so with prefix. In a simple for-loop with one million loops this can easily make a difference of 0.5 to 1 seconds (more complex data types that have an overloaded ++ operator even have a much higher impact on performance).

++i; // best practice
i++; // ONLY where not possible the other way round.

Function calls in loop conditions

Only if the code logic requires (and this is only if the condition is not constant over the time of the loop). Prefer using C++11 foreach loops or iterators when possible.

// BAD, getSize() is called on EVERY pass
for(int i = 0; i < getSize(); ++i)
{
    // code that is not changing the return value of getSize()
}

// GOOD, getSize() is called only ONCE
for(int i = 0, loopLength = getSize(); i < loopLength; ++i)
{
    // code that is not changing the return value of getSize()
}

References and const correctness

Use const references for larger objects where the argument is not intended to be copied.

// BAD, unnecessary copy of text value
void doSomeStuff(std::string text)
{
    // Code that only reads from text
}

// Good, text is not copied by value, but by reference.
void doSomeStuff(const std::string& text)
{
    // Code that only reads from text
}

Getters and Setters

In classes, use public getter and setter methods to get and set private (or protected) properties. These are also the only ones where the above style guidelines should be ignored:

Getters and setters are inline, referenced and const-correct one-liners that completly go into the header. Only more complex getters and setters are handled like "normal" functions (separate declaration and implementation, not inline).

// some header file
public:
    inline const std::string& getName() const
    { return name; }

    inline void setName(const std::string& newName)
    { name = newName; }

private:
    std:string name;

Temporary variables

If possible avoid temporary variables. Reason: No need for using memory and CPU time for something that isn't really needed. A temporary variable is only useful if it is needed often in the ''same scope'' and gives more control over the code than without it: for example if the temporary variable helps to avoid always the same function call.

/* BAD, tempFoo is only needed once,
 * no reason to use it here
 */
int tempFoo = 3 * getSomeValue();
someFunction(tempFoo);

/* GOOD, the calculation is only needed once,
 * So no reason to use a temp variable
 */
someFunction(3 * getSomeValue());

/* GOOD, the calculation is needed often,
 * so here it is useful to use a temporary
 * variable
 */
int tempFoo = 3 * getSomeValue();
someFunctionOne(tempFoo);
someFunctionTwo(tempFoo);
someFunctionThree(tempFoo);

Initialization vs. Assignment (and where to do it)

Short answer: Use direct initialization, if possible, in the scope where needed, this reduces possibly unneeded creations of variables! It's also faster than declaring and assigning in two separate steps. Additionally it makes the code more readable.

/* BAD, foo is ALWAYS created
 * BAD, declaring and assigning is slower */
int foo;
foo = getSize();
if(someCondition)
{
    someFunctionOne(foo);
    someFunctionTwo(foo);
    someFunctionThree(foo);
}

/* GOOD, foo is only created if needed,
 * GOOD, direct initialization is faster */
if(someCondition)
{
    int foo = getSize();
    someFunctionOne(foo);
    someFunctionTwo(foo);
    someFunctionThree(foo);
}

Conditional assignments (inline-if-else)

If a single assignment depends on a condition, the "?:" inline conditional can be used. It makes the code shorter and easier to read if correctly indented, most compilers also generate shorter assembly code with these. This can avoid bugs, too (accidentally using the wrong variable in if or else block, with inline conditionals however you only need to type it once, which greatly lowers the possibility of a typo).

// BAD
int a, s;
if(conditionForA)
{
    a = foo;
}
else
{
    a = bar;
    /* s = bar <- this could have been a bug
     * if you accidentally assigned to s instead of a
     */
}

// GOOD
int a, s;
a = (conditionForA) ? foo //true
    : bar;                //false

Initialization lists

Prefer initializing variables in the initializing list when possible, as this avoids calling the constructor more than once.

// BAD
Foo::Foo(int bar)
{
    a = "";
    b = 0;
    c = bar;
}

// GOOD
Foo::Foo(int bar) :
    a(""), // N.B.: std::string don't actually need to be initialized to an empty value. This is just for the example.
    b(0),
    c(bar)
{}