Skip to content

Coding Style

Jump to bottom
Tobias Markmann edited this page Jul 14, 2017 · 15 revisions

General

  • When editing a file, use the same style as the rest of the file.

  • Treat compiler warnings as errors, except on Windows. Our build will reject code that has compiler warnings in it. You can ensure you don't have warnings by setting allow_warnings = 0 in the SCons build configuration.

  • Include unit tests. Historically the project used CppUnit as testing framework, so most existing tests are written using that framework. However, new tests should be written using Google Test. For examples, see here or here.

  • Add a comment with copyright information to every file you add to the project. The copyright should be of the following form:

      /*
   * Copyright (c) 2012-2013 John Doe
   * Licensed under the Simplified BSD license.
   * See Documentation/Licenses/BSD-simplified.txt for more information.
   */

  • When in doubt, ask us

Naming

  • Always use full nouns/verbs as names, unless it is a widely accepted abbreviation, or unless it is a variable that represents an index or iterator. E.g. myThing, myJIDFactory, ...
  • Type names (class, enums, ...) are camel case, and start with a capital. E.g. MyClass, MyJIDFactory
  • Function names are camel case, and start with a lower case letter, and should be imperative. E.g. getValue, setValue. For boolean tests, use is as prefix, e.g. isValid()
  • Variable names are camel case, and start with a lower case letter. E.g. myVariable.
  • Member variables follow the same naming as variable names. We people like to add an _ to the end of member variable names, although not all classes follow this. When editing a class, use the same convention as the rest of the member variables.
  • Macro names and constants are capitalized, e.g. MY_MACRO, static const int MY_CONSTANT
  • Capitalisation of an acronym/initialism is consistent with the first letter. e.g. raidDisk (not rAIDDisk), controllerForRAID (not controllerForRaid), SASLHandler (not SaslHandler), saslHandler (not sASLHandler).
  • Source files have the extension .cpp (C++), .m (Objective-C), .mm (Objective-C++); header files have the extension .h.

Pointers

  • Never use raw pointers for ownership (which implies 'never delete a pointer')
  • If you own an object, use a unique_ptr
  • If ownership is shared (e.g. you create it, and pass it somewhere else that it might outlive you), use a shared_ptr.
  • If using references 'backwards', use a weak_ptr to avoid circular references
  • If a raw pointer is passed in to a constructor, it is same to store it because its lifetime is longer than yours
  • If you own a pointer (or have a raw pointer stored from your constructor), you can pass it into the constructor of any object that you own. The object's life is shorter than yours, and the pointer's life must be longer than yours.
  • Some old code may use raw pointers for ownership; until this is modernised, take care.
  • For shared_ptr shortcuts in classes use using ref = std::shared_ptr<Foo> instead of typedef std::shared_ptr<Foo> ref

Formatting

  • Use four spaces for indentation, never tabs. (Yes, tabs are the superior option, but the world has spoken)

  • Look throughout the source code to see which indentation style we use

  • Always use braces for control statements, even if the body only contains one statement. E.g.

      if (foo) {
    bar();
  }

    NOT

      if (foo) 
    bar();

  • Namespace indentation: in .cpp files, don't indent code inside the main namespace E.g.

      namespace Swift {

  JID::JID() {
    ...
  }

  }

    This avoids needless indentation. What also usually works is:

      using namespace Swift;

  JID::JID() {
    ...
  }

    For anonymous namespaces, it is ok to indent as usual:

      namespace {
    struct Foo {
    };
  }

  • Put the * with the type. E.g.

      Foo* myFoo;

    NOT:

      Foo *myFoo;

Includes

  • Always use #include <...> (so not #include "...")
  • Include as little as possible. Use forward declarations if you don't need the class definition (but only for classes and structs)
  • Include the header of every class you use
  • Never include the following headers in a header file:
    • iostream
    • Swiften/Base/Log.h
    • Swiften/Base/foreach.h
  • In a .cpp file, always include the header of the file you're declaring first. This avoids accidental hidden dependencies that aren't included in the header.
  • Don't include library headers in public header files (except for boost). If needed, use pimpl to hide the members of a class that depend on a library type. For an example, see Swiften/Parser/ExpatParser.h
  • Group includes together. Within a group sort them alphabetically and have a single blank line between groups. The groups should be in the order:
    • Declaring .h file, if you're in a .cpp
    • C Standard Library
    • C++ Standard library
    • Boost
    • Qt
    • Other third-party libraries (e.g. XML)
    • Swiften
    • Swift/Controllers
    • SwifTools
    • Swift/QtUI
  • There is also a script that will help with sorting and grouping the includes accordingly, see BuildTools/FixIncludes.py. Call it with -i to fix the include in place. If you find any issue with this script, please report it.

Misc

  • Use range-based for loops (rather than for(;;) with explicit iterators) where possible. Combine this with auto or const auto for pointer loop variables, and auto& or const auto& for object loop variables. Always prefer the const variant where possible, e.g.

      for (const auto&, someVector) {...

  • Always use C++11 override keyword when overriding virtual methods, e.g.

      virtual bool myOverriddenMethod() const override

  • Always use SWIFTEN_API (Swiften/Base/API.h) on Swiften classes, except for classes that need to remain private

  • Never use using namespace in headers. This pollutes the global namespace of every site that includes your header.

  • Don't use default case statements when switching on an enum. This makes sure that we get compiler warnings whenever a value is added to an enum.

  • Every class with virtual methods needs to have at least one outlined virtual method. As a rule of thumb, always putting the destructor (even an empty one) in a .cpp file ensures this. (Note: not all classes currently adhere to this rule yet).

  • In general, outline all methods in a .cpp in preference to a header.

  • Prefer pre-increment (++i) over post-increment (i++).

  • Prefer vector<> over list<>. Even when regularly inserting or removing from random places in a container, vector outperforms list (for data types that aren't too large)

  • Use assert where a failed precondition would be unrecoverable, but never use an assert based on network traffic. Don't forget to include <cassert> if you use assert.

  • If a failed precondition isn't unrecoverable, where a crash is worse than the failure, or on network data, never assert. Use SWIFT_LOG_ASSERT (Swiften/Base/Log.h) in this case.

  • All parameters passed as references should be const. If you want to break this rule, please ask us first before submitting the patch.

  • In general, declare one variable per line (not int a = 0, b = 1, c = 3...;).

  • Use C++11 features in preference to their Boost (or other) equivalents, e.g. for/foreach, std::shared_ptr, ...

  • Never (C-Style-Cast)Something, always explicitly pick a *_cast<> instead, to avoid mistakes accidentally resulting in a reinterpret_cast. Avoid reinterpret_casts, even explicitly.

Clone this wiki locally