Skip to content

Commit

Permalink
Fix markup
Browse files Browse the repository at this point in the history
  • Loading branch information
@SupSuper
SupSuper committed Mar 11, 2019
1 parent 344a16d commit 39f710a
Showing 1 changed file with 52 additions and 52 deletions.
104 changes: 52 additions & 52 deletions CODE_STYLE.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ GCCIsntMuchBetter

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 [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:

Expand Down Expand Up @@ -46,13 +46,13 @@ void function()
{
reallyLongFunctionNameWithLotsOfArguments(argOne, argTwo,
argThree);
}
}
```
* 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 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.
* When breaking a single statement, break the line before the next operator. Avoid having an operator as the last thing on a line.
```C++
void reallyLongFunctionNameIMeanThisIsReallyBadlyNamedWhateverIDontCareTheyPayMeAnyway(int parameterOne,
int paramTwo, char theThirdOne)
Expand Down Expand Up @@ -94,14 +94,14 @@ Scope:
* 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
* Functions
```C++
void functionDefinition()
{
newScopeHere();
}
```
* new conditional block (if/else/when/for)
* new conditional block (if/else/when/for)
```C++
if (x)
{
Expand All @@ -117,10 +117,10 @@ void functionDefinition()
}

```
* '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'
* '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'
```C++
switch (a)
{
Expand All @@ -138,8 +138,8 @@ void functionDefinition()
break;
}
```
* Class/enum/struct declarations
* note: public/private/protected are an exception to this, being aligned to 'class' keyword, not 'within' it's scope
* Class/enum/struct declarations
* note: public/private/protected are an exception to this, being aligned to 'class' keyword, not 'within' it's scope
```C++
class MyClass
{
Expand All @@ -150,12 +150,12 @@ public:
};
```
* 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
* 'Trivial' is defined by a single statement that fits within the 100-column limit
```C++
int Class::function() {return 0;}
```
* New scope is not caused by:
* namespace (which should also have a comment stating the namespace name at the closing bracket)
* namespace (which should also have a comment stating the namespace name at the closing bracket)
```C++
namespace OpenApoc
{
Expand All @@ -166,7 +166,7 @@ private:
};
}; // namespace OpenApoc
```
* labels
* labels
* Labels and #preprocessor directives /always/ on column 0 (start of line) no matter the scope
```C++
#if defined(LINUX)
Expand Down Expand Up @@ -214,7 +214,7 @@ public:
int function(int parameterOne, char secondOne)
```
* Variables should be camelBack
* Don't be afraid to use 'short' variable names if it's obvious
* Don't be afraid to use 'short' variable names if it's obvious
```C++
void function()
{
Expand Down Expand Up @@ -260,7 +260,7 @@ void localFunction()
}; // anonymous namespace
```
* 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.
* All "char*"/std::string params are assumed to be in utf8 format.


Templates:
Expand All @@ -280,7 +280,7 @@ 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
* Always use 'private', even if that's the default
```C++
class MyClass
{
Expand All @@ -291,7 +291,7 @@ public:
};
```
* 'virtual' keyword only used for base class, 'override' used for derived
* All classes with a virtual (or overrided) function _must_ specify a virtual destructor
* 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)
```C++
class BaseClass
Expand All @@ -307,7 +307,7 @@ public:
void someFunction() override;
};
```
* Never use both 'virtual' and 'override
* Never use both 'virtual' and 'override
* Don't define an empty {} body in the header for constructors/destructors etc. - use '= default' instead
```C++
class MyBaseClass
Expand All @@ -324,7 +324,7 @@ public:
virtual void functionBaseClassesMustOverride() = 0;
};
```
* No need for 'pure' interface classes, they can have code that all subclasses will use!
* 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)
```C++
class MyClass
Expand All @@ -334,14 +334,14 @@ public:
};
```
* In constructors prefer initialisation of members with initialiser list over assignment
* Good:
```C++
* Good:
```C++
MyClass::MyClass(Type value) : member(value)
{
doWhatever();
}
```
* Bad:
* Bad:
```C++
MyClass::MyClass(Type value)
{
Expand All @@ -350,7 +350,7 @@ MyClass::MyClass(Type value)
}
```
* Initialisers should be in order of declaration in the class
* For example with the class:
* For example with the class:
```C++
class MyClass
{
Expand All @@ -361,18 +361,18 @@ public:
MyClass(Type valueA, Type valueB);
};
```
* Good:
* Good:
```C++
MyClass::MyClass(Type valueA, Type valueB) : memberA(valueA), memberB(valueB) {}
```
* Bad:
* Bad:
```C++
MyClass::MyClass(Type valueA, Type valueB) : memberB(valueB), memberA(valueA) {}
```
* 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)
* 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)
```C++
struct DataFileSection
{
Expand All @@ -381,12 +381,12 @@ struct DataFileSection
uint32_t z;
};
```
* If using a struct to read in data, use a static_assert to ensure correct sizing:
* If using a struct to read in data, use a static_assert to ensure correct sizing:
```C++
static_assert(sizeof(DataFileSection) == 12, "DataFileSection not expected 12 bytes");
```
* 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 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.
Expand Down Expand Up @@ -415,43 +415,43 @@ General code:
```C++
auto varableName = function();
```
* Note where auto& may be better to avoid a copy
* Note where auto& may be better to avoid a copy
* sp<> up<> wp<> smart pointers
* make_shared() instead of new
```
* make_shared() instead of new
```C++
auto var = std::make_shared<Type>(args);
```
* 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
* 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
```C++
up<Type> var{new Type{args}};
functionThatTakesOwnershipOfParam(std::move(var));
```
* 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
* Unless you explicitly want to copy an object in
* Use foreach loops where possible ( "for (auto &element : container)")
```C++
for (auto &element : container)
{
whatever(element);
}
```
* Exception may be a 'safe' iterator when possibly removing elements during loop, then use iterator and keep copy locally
* 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
* '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
* 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
```C++
MyClass classInstance{argumentOne, argTwo};
```
* Requires you to avoid implicit conversions - this is good!
* Requires you to avoid implicit conversions - this is good!
* STL initialiser lists are good
```C++
std::vector<int> someInts = {1, 2, 3};
Expand All @@ -471,7 +471,7 @@ General code:
doLotsOfBoringStuff();
```
* 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 :)
* Note limitations WRT goto: over stack initialisers - IE you can't do it :)
```C++
for (auto &x : containerX)
Expand All @@ -483,7 +483,7 @@ General code:
goto end;
}
}
}
}
end:
return;
```
Expand All @@ -504,26 +504,26 @@ Comments:
* last line */
```
* 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
* 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"
* 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
* do forward declaration of classes instead where possible
```C++
class ForwardDeclaredType;

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

0 comments on commit 39f710a

Please sign in to comment.