Skip to content
Evan Teran edited this page Oct 29, 2019 · 42 revisions

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.

Language Features

edb is written using C++14. We may choose to move on to C++17 sometime in the future, but for now, please stick to features that are provided by C++14 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.

Warnings

The goal is always to compile without warnings when built with -std=c++14 -W -Wall -pedantic.

Portability

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.

Namespace using directives

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.

auto keyword

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

nullptr keyword

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.

Functions that return pointers (and similar)

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* ...
}

Braces & Newlines

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);
}

Spacing around operators

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

Virtual Functions

Virtual functions should have exactly one of virtual, override, final.

struct Foo {
    virtual void bar(); // GOOD
}

struct Baz {
    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
}

Naming

There is a lot of inconsistency in the codebase that has developed over the years as personal tastes, conventions, and best practices have evoled 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

      class MyClass {};
    
  • Member Functions - Should follow camelCase

      class 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() {}
    
  • Deviation from this style is permitted for code which is trying to be compatible with existing APIs found in the standard library.

Clone this wiki locally