Coding Style (OpenApoc) - Revision history

Deldonut1: Re-add 9 coding conventions from CODE_STYLE.md that were missing: comments style, trivial one-line functions, ownership guidance, static_assert, iterator exception, forward declaration limits, Yoda conditionals, numeric_limits, short variable names

2026-03-01T23:05:58Z

Re-add 9 coding conventions from CODE_STYLE.md that were missing: comments style, trivial one-line functions, ownership guidance, static_assert, iterator exception, forward declaration limits, Yoda conditionals, numeric_limits, short variable names

← Older revision		Revision as of 23:05, 1 March 2026
Line 137:		Line 137:
	}		}
	</pre>		</pre>

			* '''Exception''': Trivial functions (a single statement fitting within 100 columns) may use the definition and body on one line:

			<pre>
			int getX() const { return x; }
			</pre>

			This is enforced by <code>clang-format</code> (<code>AllowShortFunctionsOnASingleLine: All</code>).

	== Naming ==		== Naming ==
Line 151:		Line 159:
	\| <code>lower_case</code> \|\| Labels		\| <code>lower_case</code> \|\| Labels
	\|}		\|}

			Short variable names (<code>i</code>, <code>x</code>, <code>y</code>) are fine when the meaning is obvious from context.

	== Types ==		== Types ==
Line 159:		Line 169:
	* No C-style casts — use <code>static_cast<></code>, <code>dynamic_cast<></code>, <code>reinterpret_cast<></code>		* No C-style casts — use <code>static_cast<></code>, <code>dynamic_cast<></code>, <code>reinterpret_cast<></code>
	* Prefer <code>{}</code> brace initialization		* Prefer <code>{}</code> brace initialization
			* Prefer <code>std::numeric_limits<></code> over C-style <code>MAX_INT</code> / <code>INT_MAX</code> macros

	== Smart Pointers ==		== Smart Pointers ==
Line 181:		Line 192:
	* Prefer <code>up<></code> (exclusive ownership) over <code>sp<></code> unless shared ownership is genuinely needed		* Prefer <code>up<></code> (exclusive ownership) over <code>sp<></code> unless shared ownership is genuinely needed
	* Use <code>std::move()</code> to transfer <code>up<></code> ownership		* Use <code>std::move()</code> to transfer <code>up<></code> ownership
			* '''Ownership guideline''': If ownership of a member is tied to the class, use a direct member (no pointer). If a reference owned by another object is guaranteed to outlive the class, use a <code>&reference</code> member. Otherwise use <code>up<></code> or <code>sp<></code> depending on whether ownership is exclusive or shared.

	== Templates ==		== Templates ==
Line 225:		Line 237:
	* '''<code>const</code> aggressively''': const member functions, const parameters, const return types, const local variables		* '''<code>const</code> aggressively''': const member functions, const parameters, const return types, const local variables
	* Range-for loops: <code>for (auto &element : container)</code> / <code>for (const auto &element : container)</code>		* Range-for loops: <code>for (auto &element : container)</code> / <code>for (const auto &element : container)</code>
			** '''Exception''': When removing elements during iteration, use an iterator-based loop instead of range-for
	* Prefer <code>emplace()</code> over <code>insert()</code> in STL containers		* Prefer <code>emplace()</code> over <code>insert()</code> in STL containers

Line 254:		Line 267:
	} // namespace OpenApoc		} // namespace OpenApoc
	</pre>		</pre>

			* Use <code>static_assert()</code> to verify assumptions about alignment, packing (e.g., when reading structs from original game files), or template restrictions
			* Avoid Yoda conditionals — write <code>var == 1</code>, not <code>1 == var</code>

			== Comments ==

			* Prefer <code>//</code> for single-line comments
			* Multi-line <code>/* /</code> comments: align the <code></code> on each intermediate line
			* Use <code>//TODO:</code> and <code>//FIXME:</code> markers to flag work-in-progress or known issues
			* Do not comment for the sake of it — if the code can be made clearer, prefer that over adding a comment

	== Logging ==		== Logging ==
Line 295:		Line 318:
	* Local headers use paths '''relative to the OpenApoc root''': <code>"framework/logger.h"</code>, not <code>"../logger.h"</code> or <code>"logger.h"</code>		* Local headers use paths '''relative to the OpenApoc root''': <code>"framework/logger.h"</code>, not <code>"../logger.h"</code> or <code>"logger.h"</code>
	* '''Prefer forward declarations''' over <code>#include</code> in headers where possible		* '''Prefer forward declarations''' over <code>#include</code> in headers where possible
			** Forward declarations are '''not possible''' for templates, non-class types (e.g., <code>enum class</code>), and superclasses. When in doubt, try building without the include and see what fails.

	<pre>		<pre>

Deldonut1: Major rewrite: updated to C++17/clang-format-18; added smart pointers; logging; strings; review expectations

2026-03-01T21:01:43Z

Major rewrite: updated to C++17/clang-format-18; added smart pointers; logging; strings; review expectations

Show changes

SupSuper at 19:58, 9 March 2019

2019-03-09T19:58:22Z

← Older revision		Revision as of 19:58, 9 March 2019
Line 485:		Line 485:
	Comments:		Comments:
	* either // or /* */ is fine - prefer // for single line		* either // or /* */ is fine - prefer // for single line
	* If doing multi line /\-style comments have an aligned '\' with at the beginning of each subseqent line:		* If doing multi line /-style comments have an aligned '' with at the beginning of each subseqent line:
	<pre>		<pre>
	/* first line		/* first line

SupSuper at 19:57, 9 March 2019

2019-03-09T19:57:18Z

← Older revision		Revision as of 19:57, 9 March 2019
Line 78:		Line 78:
	if (a == 0)		if (a == 0)
	</pre>		</pre>
	* References and pointers: & and** align to right (to variable) not type		* References and pointers: & and * align to right (to variable) not type
	<pre>		<pre>
	float**pointerToFloat;		float *pointerToFloat;
	</pre>		</pre>

Line 484:		Line 484:

	Comments:		Comments:
	* either // or /***/ is fine - prefer // for single line		* either // or /* */ is fine - prefer // for single line
	* If doing multi line /\-style comments have an aligned '\' with at the beginning of each subseqent line:		* If doing multi line /\-style comments have an aligned '\' with at the beginning of each subseqent line:
	<pre>		<pre>
	/* first line		/* first line
	** second line		* second line
	last line/		* last line */
	</pre>		</pre>
	* Don't comment for the sake of it		* Don't comment for the sake of it

SupSuper at 19:29, 9 March 2019

2019-03-09T19:29:20Z

← Older revision		Revision as of 19:29, 9 March 2019
Line 13:		Line 13:
	C++11 features are heavily encouraged - patterns from 'older' c++ versions that have been superceded should be avoided.		C++11 features are heavily encouraged - patterns from 'older' c++ versions that have been superceded should be avoided.

	The formatting sections of this document are enforced by the ~~``clang-format`` tool~~ [http://llvm.org/releases/4.0.0/tools/clang/docs/ClangFormat.html]. Currently the '4.0' version of ``clang-format`` is to be used. The configuration file ``.clang-format`` in the root of the OpenApoc source repository should match the formatting guidelines specified below.		The formatting sections of this document are enforced by the [http://llvm.org/releases/4.0.0/tools/clang/docs/ClangFormat.html clang-format tool]. Currently the '4.0' version of <code>clang-format</code> is to be used. The configuration file <code>.clang-format</code> in the root of the OpenApoc source repository should match the formatting guidelines specified below.

	With this, it is highly recommended to run ``clang-format`` on all modified files before check-in. This can be run on source files with the following command:		With this, it is highly recommended to run <code>clang-format</code> on all modified files before check-in. This can be run on source files with the following command:

	<pre>		<pre>
Line 21:		Line 21:
	</pre>		</pre>

	When ran from the root of the OpenApoc source repository it should automatically use the supplied ``.clang-format`` configuration file. The tool also supports modifying the supplied source files to match the configured format when passed the ``-i`` flag:		When ran from the root of the OpenApoc source repository it should automatically use the supplied <code>.clang-format</code> configuration file. The tool also supports modifying the supplied source files to match the configured format when passed the <code>-i</code> flag:

	<pre>		<pre>
Line 27:		Line 27:
	</pre>		</pre>

	When using the ``CMake`` build system (as used on Unix-based platforms), there is a ``format-sources`` make target that will run ``clang-format -i`` on all source files within the OpenApoc repositry tree. This provides a single command that can be run before commiting:		When using the <code>CMake</code> build system (as used on Unix-based platforms), there is a <code>format-sources</code> make target that will run <code>clang-format -i</code> on all source files within the OpenApoc repositry tree. This provides a single command that can be run before commiting:

	<pre>		<pre>
Line 33:		Line 33:
	</pre>		</pre>

	In the future, it may be possible to use ``clang-tidy``, also from the clang project, to enforce more of the non-formatting sections of this style guideline. One example of this is an identifier name checking tool.		In the future, it may be possible to use <code>clang-tidy</code>, also from the clang project, to enforce more of the non-formatting sections of this style guideline. One example of this is an identifier name checking tool.

SupSuper: Created page with "This document specifies the guidelines for writing and formatting the c++ code that forms the core of OpenApoc. Globally, we use 'standard' c++11. This requires reasonably mo..."

2019-03-09T19:24:33Z

Created page with "This document specifies the guidelines for writing and formatting the c++ code that forms the core of OpenApoc. Globally, we use 'standard' c++11. This requires reasonably mo..."

New page

This document specifies the guidelines for writing and formatting the c++ code that forms the core of OpenApoc.

Globally, we use 'standard' c++11. This requires reasonably modern compilers (gcc 4.7+, MSVC 2013+ , clang 3.6+ have been tested). You should avoid using compiler-specific stuff where possible. Exceptions to this exist, but should be wrapped in a #ifdef:
<pre>
#ifdef _MSC_VER
MSVCIsCrazySometimes
#else
// Everything else we support (gcc + clang) are pretty much extension-compatible
GCCIsntMuchBetter
#endif
</pre>

C++11 features are heavily encouraged - patterns from 'older' c++ versions that have been superceded should be avoided.

The formatting sections of this document are enforced by the ``clang-format`` tool [http://llvm.org/releases/4.0.0/tools/clang/docs/ClangFormat.html]. Currently the '4.0' version of ``clang-format`` is to be used. The configuration file ``.clang-format`` in the root of the OpenApoc source repository should match the formatting guidelines specified below.

With this, it is highly recommended to run ``clang-format`` on all modified files before check-in. This can be run on source files with the following command:

<pre>
clang-format path/to/file.cpp path/to/file.h
</pre>

When ran from the root of the OpenApoc source repository it should automatically use the supplied ``.clang-format`` configuration file. The tool also supports modifying the supplied source files to match the configured format when passed the ``-i`` flag:

<pre>
clang-format -i path/to/file.cpp path/to/file.h
</pre>

When using the ``CMake`` build system (as used on Unix-based platforms), there is a ``format-sources`` make target that will run ``clang-format -i`` on all source files within the OpenApoc repositry tree. This provides a single command that can be run before commiting:

<pre>
make format-sources
</pre>

In the future, it may be possible to use ``clang-tidy``, also from the clang project, to enforce more of the non-formatting sections of this style guideline. One example of this is an identifier name checking tool.

== Indent ==
* Tabs for indenting, spaces for alignment, indenting by 1 tab for each new scope
<pre>
void function()
{
reallyLongFunctionNameWithLotsOfArguments(argOne, argTwo,
argThree);
}
</pre>
* Avoid going over 100 cols (at tab width of '4' spaces).
** If you find yourself going over this it's often a hint to try to pull things out of loops/into functions
** Don't break strings up to fit this, it looks ugly and makes things even harder to read.
* If you have to break, indent the following line by an extra tab
** When breaking a single statement, break the line before the next operator. Avoid having an operator as the last thing on a line.
<pre>
void reallyLongFunctionNameIMeanThisIsReallyBadlyNamedWhateverIDontCareTheyPayMeAnyway(int parameterOne,
int paramTwo, char theThirdOne)
{
if (parameterOne == yetAnotherReallyLongLineHereComeSomeWordsBlaBlaBlaAreYouStillReadingThisComeOnDont
&& youHaveBetterThingsToDo)
{
doWhatever();
}
}
</pre>

== Whitespace ==
* Spaces before and after operators
<pre>
a = b;
a && b;
a + b;
</pre>
* Space after if/when/else/for, space after :/; in for
<pre>
for (auto &a: b)
</pre>
* No spaces after function name (or function-like keywords like 'sizeof'), but space after flow control keywords, space after comma for multiple args
<pre>
func(a, b);
if (a == 0)
</pre>
* References and pointers: & and** align to right (to variable) not type
<pre>
float**pointerToFloat;
</pre>

== Scope ==
* Indent 1 tab for each new scope
* New scope is _always_ surrounded by {} braces
* New scope has a '{' on the /next/ line at the indent of the old scope(not the new scope)
* closing scope '}' same indent at opening '{', again on a new line
* New scope caused by:
** Functions
<pre>
void functionDefinition()
{
newScopeHere();
}
</pre>
** new conditional block (if/else/when/for)
<pre>
if (x)
{
doWhatever();
}
else if (y)
{
doWhateverTheSecond();
}
else
{
doThatLastThing();
}

</pre>
** 'switch'
** 'case' also indents a new scope. {} are optional, based on if new stack variables are needed to handle the case.
*** Note switch should always have a default case unless over an enum class (where they may not if (and only if) every value is handled)
*** All 'case' sections should have a 'break'
<pre>
switch (a)
{
case A:
doSomething();
break;
case B:
{
auto var = somethingElse();
doSomethingElse(var);
break;
}
default:
doDefaultCase();
break;
}
</pre>
** Class/enum/struct declarations
*** note: public/private/protected are an exception to this, being aligned to 'class' keyword, not 'within' it's scope
<pre>
class MyClass
{
private:
int privateVariable;
public:
void publicFunction();
};
</pre>
* Exception to this is 'trivial' functions that have the definition & contents all on line line
** 'Trivial' is defined by a single statement that fits within the 100-column limit
<pre>
int Class::function() {return 0;}
</pre>
* New scope is not caused by:
** namespace (which should also have a comment stating the namespace name at the closing bracket)
<pre>
namespace OpenApoc
{
Class MyClass
{
private:
int x;
};
}; // namespace OpenApoc
</pre>
** labels
* Labels and #preprocessor directives /always/ on column 0 (start of line) no matter the scope
<pre>
#if defined(LINUX)
x = linuxFunction();
#else
x = otherFunction();
#endif
if (x)
{
goto error;
}
error:
return 0;
</pre>

== Case ==
* Namespaces should be CamelCase
<pre>
namespace OpenApoc
</pre>
* Classes and enums should be CamelCase
<pre>
class MyClass
</pre>
* 'enum class' members should be CamelCase
<pre>
enum class MyEnum
{
ValueOne,
ValueTwo,
};
</pre>
* class methods and member variables (public/private/protected) should be camelBack
<pre>
class MyClass
{
public:
void someFunction();
int someVariable;
};
</pre>
* Function parameters (public/private/protected) should be camelBack
<pre>
int function(int parameterOne, char secondOne)
</pre>
* Variables should be camelBack
** Don't be afraid to use 'short' variable names if it's obvious
<pre>
void function()
{
int localVariable = 0;
int x = 1;
int y = 5;
for (int i = 0; i < 5; i++)
</pre>
* class/global constants should be SHOUTY_CAPS, along with _all_ macros
<pre>
#define OPENAPOC_VERSION "1.0"
</pre>
* Labels should be lower_case:
<pre>
exit_loop:
goto exit_loop;
</pre>
* All members should be initialised in all constructors if they don't have a default constructor. You can use member initialisation in the class definition if this is clearer
<pre>
class MyClass
{
int variableMember = 0;
};
</pre>

== Types ==
* Avoid typedef - use 'struct' keyword where necessary in c-like code
<pre>
struct MyStruct
{
int x;
};

void myStructUser(struct MyStruct s)
</pre>
* up<> sp<> wp<> aliases are defined for std::unique_ptr<>, std::shared_ptr<>, std::weak_ptr<> in "library/sp.h". Use them instead of the verbose versions.
* Use anonymous namespaces for 'file-local' stuff (instead of static, as you can wrap classes in it too)
<pre>
namespace
{
void localFunction()
}; // anonymous namespace
</pre>
* We provide a "UString" class. This should be used for _all_ strings, as it provides platform-local non-ascii charset handling
** All "char*"/std::string params are assumed to be in utf8 format.

== Templates ==
* If templates help, go ahead, don't avoid them
* prefer 'typename' to 'class'
* template types should be CamelCase
<pre>
template <typename LocalType> function(LocalType param)
</pre>
* 'short' typenames are OK if it's obvious what's going on
<pre>
template <typename T> Class<T>::function()
</pre>

== Class declarations ==
* member functions camelCase()
* 'public:' 'private:' 'protected:' are indented to the 'class' keyword, everything within them indented to class scope.
** Always use 'private', even if that's the default
<pre>
class MyClass
{
private:
int localVariable;
public:
void publicFunction();
};
</pre>
* 'virtual' keyword only used for base class, 'override' used for derived
** All classes with a virtual (or overrided) function _must_ specify a virtual destructor
* Inheritence should be on the same line as the 'class' keyword (until you get to the column limit and break)
<pre>
class BaseClass
{
public:
virtual ~BaseClass();
virtual void someFunction();
};

class SubClass : public BaseClass
{
public:
void someFunction() override;
};
</pre>
** Never use both 'virtual' and 'override
* Don't define an empty {} body in the header for constructors/destructors etc. - use '= default' instead
<pre>
class MyBaseClass
{
public:
virtual ~MyBaseClass() = default;
};
</pre>
* define pure virtual "virtual void func() = 0;" for 'interface' classes
<pre>
class MyInterface
{
public:
virtual void functionBaseClassesMustOverride() = 0;
};
</pre>
** No need for 'pure' interface classes, they can have code that all subclasses will use!
* For trivial initial values prefer initialisers in the class declaration (It's easier to see what's set and cleans up constructor definitions)
<pre>
class MyClass
{
public:
Type initialisedMember = 0;
};
</pre>
* In constructors prefer initialisation of members with initialiser list over assignment
** Good:
<pre>
MyClass::MyClass(Type value) : member(value)
{
doWhatever();
}
</pre>
** Bad:
<pre>
MyClass::MyClass(Type value)
{
member = value;
doWhatever();
}
</pre>
* Initialisers should be in order of declaration in the class
** For example with the class:
<pre>
class MyClass
{
public:
Type memberA;
Type memberB;

MyClass(Type valueA, Type valueB);
};
</pre>
** Good:
<pre>
MyClass::MyClass(Type valueA, Type valueB) : memberA(valueA), memberB(valueB) {}
</pre>
** Bad:
<pre>
MyClass::MyClass(Type valueA, Type valueB) : memberB(valueB), memberA(valueA) {}
</pre>
* Use 'struct' for 'data-only' types
** Structs should _never_ have public/private/protected declarations, if there's anything non-public you shouldn't use a struct.
** Likely only going to be used within data reading/writing to files
*** Because of this you're probably going to need to use 'sized' typed (see <cstdint> header)
<pre>
struct DataFileSection
{
uint32_t x;
uint32_t y;
uint32_t z;
};
</pre>
** If using a struct to read in data, use a static_assert to ensure correct sizing:
<pre>
static_assert(sizeof(DataFileSection) == 12, "DataFileSection not expected 12 bytes");
</pre>
* If ownership of a member is tied to the class, don't use a pointer and new/delete in constructor/destructor. Just use the type and initialise it correctly in the init list before the constructor.
** If the above is not possible (e.g. complex init requirements, 'may be invalid and null' use a up<>
* If we /know/ a member reference owned by another object will be live for the duration of the class, use a &reference member
* Otherwise use a sp<>/up<> depending if it should take a reference and if having a 'null' object makes sense.

== Functions ==
* Const functions where possible (IE not modifying any members)
<pre>
class MyClass
{
public:
int dataAccessor() const;
};
</pre>
* Const params where logically not to be modified
<pre>
void function(const Type& readOnlyParam)
</pre>
* Const returns where the caller should never modify
<pre>
const Type& functionWhereYouCantModifyMyReturnThanks()
</pre>

== General code ==
* Where possible use auto
<pre>
auto varableName = function();
</pre>
** Note where auto& may be better to avoid a copy
* sp<> up<> wp<> smart pointers
** make_shared() instead of new
<pre>
auto var = std::make_shared<Type>(args);
</pre>
** make_unique() would be nice, but a c++14 feature (may restrict compiler compatibility?), so you have to wrap
*** Use std::move to transfer up<> ownership
<pre>
up<Type> var{new Type{args}};
functionThatTakesOwnershipOfParam(std::move(var));
</pre>
* Never use a 'naked' new - they should always be packaged immediately in a smart pointer
* Use emplace() in STL containers where possible over insert()
** Unless you explicitly want to copy an object in
* Use foreach loops where possible ( "for (auto &element : container)")
<pre>
for (auto &element : container)
{
whatever(element);
}
</pre>
** Exception may be a 'safe' iterator when possibly removing elements during loop, then use iterator and keep copy locally
* Where possible use 'enum class'
* Naming variables - don't be afraid of using 'short' names ('i') if it's use is obvious
* While 'descriptive' names are nice, shorter code is also nice. Don't repeat context
** 'x' is fine is we already know we're doing something in coordinate space, no need to name it theXCoordinateOfTheCityMapInTiles
* Reading code is important - try to make it flow
** avoid 'yoda conditionals' (1 == var) don't help, modern compilers catch a =/== typo easily
** if post increment (x++) flows better use that, don't try to 'optimise' away the copy - the compiler will do that for you
* The compiler is more clever than you could ever possibly hope to be. Write things to be clear and obvious. Only /after/ it's proven to be a problem to you even look at optimisation (then _always have numbers_)
* don't use 'c' casts ("(int)x") - that does different things on if the object type has a defined conversion or not - use static_cast<>/reinterpret_case<> instead
* prefer {} constructor calls where possible
<pre>
MyClass classInstance{argumentOne, argTwo};
</pre>
** Requires you to avoid implicit conversions - this is good!
* STL initialiser lists are good
<pre>
std::vector<int> someInts = {1, 2, 3};
</pre>
* static_assert() any assumptions WRT alignment/packing (when reading structs from files, for example) - or any template restrictions (e.g. if this is only valid on unsigned types, check it!)
* <limits> is preferred to 'c' MAX_INT/whatever
<pre>
auto maxInt = std::numeric_limits<int>::max();
</pre>
* RAII where possible
* Early-return is cool, go ahead
<pre>
if (dontHaveToDoAnything)
{
return;
}
doLotsOfBoringStuff();
</pre>
* goto: is useful in some specific cases (e.g. breaking out of nested loops) - but only use it where another keyword won't do what you want
** Note limitations WRT goto: over stack initialisers - IE you can't do it :)
<pre>

for (auto &x : containerX)
{
for (auto &y : x.containerY)
{
if (weShouldStopAt(y))
{
goto end;
}
}
}
end:
return;
</pre>

== Logging ==
* LogInfo/LogWarning/LogError take 'printf-style' format string. Make sure everything is the correct type, %d for ints, %u for unsigned, %s for "utf8" const char* "strings" or UString objects directly
* LogInfo is cheap - use it everywhere interesting
* LogWarning should be something that has gone wrong, but recoverable.
* LogError is for fatal errors.

Comments:
* either // or /***/ is fine - prefer // for single line
* If doing multi line /\*-style comments have an aligned '\*' with at the beginning of each subseqent line:
<pre>
/* first line
** second line
** last line**/
</pre>
* Don't comment for the sake of it
** Try to make the code clearer first if a comment is 'required' to make something obvious
** Function/variable names are useful here - if reading it aloud describes what your comment was going to say that's perfect
** //TODO: //FIXME: when leaving known breakage

== Includes ==
* "local.h" files first
* then <system.h> includes
* Within each of the 2 blocks try to keep them alphabetically sorted (some exceptions may happen, if there's a system dependency not managed by the system header itself)
* local headers always relative to the root of OpenApoc - even if in the same directory
** e.g. "framework/event.h" not "../event.h" or "event.h"

== Headers ==
* prefer "#pragma once" to "#ifndef __HEADER_NAME" include guards
* Headers should avoid #include "dependency.h" where possible
** do forward declaration of classes instead where possible
<pre>
class ForwardDeclaredType;

void someFunction(ForwardDeclaredType &param);
</pre>
** 'not possible' includes templates, non-class types, superclasses, try building it without and see what fails

[[Category:OpenApoc]]