-
-
Notifications
You must be signed in to change notification settings - Fork 327
Style Guide
Here you will find the style guide for edb. If you are considering submitting a PR, please do you best to meet these guidelines. It is worth noting that edb has been in development for a while now, and has had its style change over time, and therefore is currently is non-compliant with this guide and that we are working to resolve that.
edb is written using C++17. We may choose to move on to C++20 sometime in the future, but for now, please stick to features that are provided by C++17 only. So long as a feature being used is well supported by both GCC and Clang, unless otherwise noted in the guide, it is acceptable to utilize it.
The goal is always to compile without warnings when built with -std=c++14 -W -Wall -pedantic.
Code should be standards compliant and not use compiler extensions (exceptions can be made, but must be wrapped in compiler-specific #ifdef blocks. Additionally, it should avoid undefined behavior at all times.
Note: C++11 introduced a syntax for type aliases which make use of the using keyword, this is not talking about those.
Never place a namespace using directive in a header.
Prefer explicit namespace usage such as:
std::string s;
You may utilize a using directive at function scope if it makes the code more readable, and is actually recommended if this aids in ADL. The most common example of this is with custom swap methods. For example:
class T {
public:
void swap(T &other) {
using std::swap;
swap(x, other.x);
swap(y, other.y);
}
private:
int x;
N::B y;
};
Due to ADL, if N::swap exists, it will be preferred over std::swap when swapping y.
Use when the type is already obvious on the line of code. Additionally, since iterators all offer similar interfaces for use, and tend to have stupidly long names, auto is a good fit. Additionally, there are some functions which "make" a type (deducing the type from template arguments to avoid explicit specification); auto is good for these as well.
auto it = c.begin(); // GOOD, works for any container and we all know that it's an iterator
auto p = new T(); // GOOD, we know p is of type T*, no need to write it twice
auto p = std::make_unique<T>(); // GOOD, we know p is of type std::unique_ptr<T>, no need to write it twice
auto p = std::make_shared<T>(); // GOOD, we know p is of type std::shared_ptr<T>, no need to write it twice
auto p = std::make_pair(1, 'A'); // GOOD, we know p is of type std::pair<int, char>, no need to write it twice
auto x = static_cast<int>(y); // GOOD, "int" is already obvious on this line, don't write it twice
auto f = []() { /* body */ }; // GOOD, stores the lambda function with no overhead (std::function has a runtime cost), and it isn't possible to know the "real" type of the lambda here.
auto x = func(); // BAD, no clue what the type of x is without more context
Use nullptr instead of NULL or 0. However, when testing for "null-ness", it is recommended to use boolean syntax such as this:
if (p) {
// not null
}
if (!p) {
// is null
}
This is short and concise.
If you find yourself writing code like this:
T *p = func();
if (p) {
// ...
}
It is better to place the variable declaration and assignment inside the if itself, like this:
if (T *p = func()) {
// ...
}
This works especially well for chains of downcast tests. For example:
if (auto p = dynamic_cast<A*>(base)) {
// base is of type A* ...
} else if (auto p = dynamic_cast<B*>(base)) {
// base is of type B* ...
} else if (auto p = dynamic_cast<C*>(base)) {
// base is of type C* ...
}
The preferred way, is similar K&R style, which is to put the opening brace last on the line, and put the closing brace first (This applies to all blocks (if, switch, for, while, do) including function and class definitions):
void f(int x, int y) {
if (x) {
// ...
}
switch(y) {
case 1:
// ...
break;
default:
// ...
break;
}
do {
body of do-loop
} while (condition);
}
To maximize readability, the following rules apply:
Unary operators should have no space between it and its operand Binary operators should have space on both sides.
-- x; // BAD
x ++; // BAD
a=b+c*10; // BAD
--x; // GOOD
x++; // GOOD
a = b + c * 10 // GOOD
Indent with tabs, align with spaces.
control flow keywords should be followed by a space.
if(x) // BAD
if (x) // GOOD
for (;;) // GOOD
while (x) // GOOD
Virtual functions should have exactly one of virtual, override, final.
struct Foo {
virtual void bar(); // GOOD
}
struct Baz : Foo {
void bar() override; // GOOD
void bar() final; // GOOD
virtual void bar() override; // BAD "virtual" is redundant when "override" is here
virtual void bar() final; // BAD "virtual" is redundant when "final" is here
}
There is a lot of inconsistency in the codebase that has developed over the years as personal tastes, conventions, and best practices have evolved over the years. So while the code does not currently obey these, this is what I'd like to move the code towards in the future:
-
Class Names - Should follow
PascalCase, with Acronyms only having the first letter capitalized.class MyClass {}; // GOOD class HTMLReader {}; // BAD class HtmlReader {}; // GOOD -
Member Functions - Should follow
camelCaseclass MyClass { public: void myFunction() const; }; -
Private Member Variables - Should follow
camelCase, but with a trailing underscore.class MyClass { // ... private: int memberVariable_; }; -
Free functions - Should follow
snake_case.void my_free_function() {} -
Macros -
ALL_CAPS, words separated by underscored#define MY_MACRO 1234 -
Constant Expressions - Should follow
PascalCase, with Acronyms only having the first letter capitalized.enum { SomeConstant = 42 }; constexpr int AnotherConstant = 42; -
Local variables, and parameters are currently inconsistent, there will be a rule reflecting this soon.
-
Deviation from this style is permitted for code which is trying to be compatible with existing APIs found in the standard library.
Any variables which exist at file scope should be placed in an anonymous namespace as opposed to using the static keyword.
static int a = 5; // BAD
static int foo_bar(); // BAD
// GOOD
namespace {
int a = 5;
int foo_bar();
}
We use the traditional macro-based include guard. The format we use is:
{OPTIONAL_PREFIX}_{FILE_BASENAME}_{FILE_SUFFIX}_{DATESTRING}_
in all upper case. Where the file's basename is changed from CamelCase to UPPER_SNAKE_CASE. So given a file named MyAwesomeFile.h, the include guards would look like this:
#ifndef MY_AWESOME_FILE_H_20191119_
#define MY_AWESOME_FILE_H_20191119_
// .. header contents
#endif
The optional prefix is used to avoid name collisions (unlikely due to the date suffix, but it happens). A good candidate would be an UPPER_SNAKE_CASE version of part of the path for the file or plugin name.