Skip to content

Style Guidelines

srh edited this page Oct 15, 2013 · 17 revisions

This document describes some coding style guidelines. Some others are covered by the first basic rule, and they haven't been broken yet, so nobody's bothered putting them here.

The Basics

  • Write your code the same way other code is written.
  • Make choices that lower the probability of mistakes.

Explicit Suggestions

  • Do not use C-style casts. Use static_cast for casting integers, casting to and from void pointers, and casting between super- and subclasses. Use reinterpret_cast for unsafe pointer reinterpretation casts.

    C-style casts can discard const information.

    reinterpret_cast does not behave correctly when casting between superclass and subclass.

  • Put spaces after if, while, for, etc. It's "if (...)", not "if(...)".

  • Put whitespace around infix operators and after commas.

    Use good_t<T, U>, not bad_t<T,U>. Make sure to use DEBUG_ONLY(arg, ), not DEBUG_ONLY(arg,), if only to satisfy scripts/check_style.sh.

  • Use full paths in #include lines, e.g. #include "btree/node.hpp", not #include "node.hpp".

    This is more readable, decreases the chance of accidentally including the wrong file, and scripts/process_headers.pl relies on full paths.

  • Includes should be in a standard order: the corresponding header file (if this is a cc file), system C headers, system C++ headers, the boost headers, and local include files, ideally in alphabetic order, with space between sections. If there are boost headers, the boost headers must have the local header file "errors.hpp" included before them, to make boost assertions go through our error system.

    Putting the corresponding header file first ensures that it includes all of its dependencies.

    An example, from the imaginary file foo/bar.cc:

#include "foo/bar.hpp"

#include <math.h> #include <stdarg.h>

#include

#include "errors.hpp" #include <boost/bind.hpp> #include <boost/function.hpp>

#include "arch/arch.hpp" #include "concurrency/pmap.hpp" #include "utils.hpp"


* Do not indent the body of namespaces.

* The end of a large namespace body should be marked with a comment:

namespace foo { // ... // 100 lines of code here // ... } /* namespace foo */

namespace bar { // ... // 100 lines of code here // ... } // namespace bar


* Output parameters should go at the end of argument lists and have "out" in their name.

Example:

bool make_foo(uint32_t quux, uint32_t baz, foo_t *foo_out);


> This refers to functions that *overwrite* the output parameter, not `memcpy`-like functions that copy to an array, or other functions that modify an existing value (possibly by appending data, which could be regarded as a form of output, but that's not what an output parameter is).

* Many classes should be non-copyable.  Put a `DISABLE_COPYING` directive inside.

> Any time copying would be a bug, this is a good idea.

* Don't use non-const references except in some return values.  Don't make member variables be references.

> Non-const references and member variable references cause mistakes to happen.

* Remember to pass vectors, shared pointers, and other such things by const reference, not by value.

> Copying all three of these things is expensive.

* Make single-argument constructors be `explicit`.

* Use NULL, not 0.

> C++11's `nullptr` isn't supported by gcc 4.4.

* Explicitly compare pointers and integers to `NULL` or zero, instead of using boolean operators or conversions.

> `if (p != NULL)` is better than `if (p)`.  `if (p == NULL)` is better than `if (!p)`.  `n == 0` is better than `!n`.  `b == true` is redundant when `b` is boolean and `b == false` should be `!b`.

* Make compilation fast.

> Try to avoid putting implementations into header files unless performance demands require it.
> Try to avoid including boost headers in header files.
> Sometimes even use a pointer instead of a flat member, so that you don't have to include the parent header file.

* Use effortful spelling, punctuation, and grammar in your comments.

> This makes them more readable.

> Don't bloviate verbiage. See also http://www.amazon.com/Style-Clarity-Chicago-Writing-Publishing/dp/0226899152

* Avoid bad abbreviations in identifiers, but good abbreviations are okay.

> The goal of abbreviating is to make code *more* readable, it's not to save on typing.

> `num_chars` is good, but `nchars` is bad.  (Or is it?)

> `element_count` is good, and `element_cnt` is bad.

> Don't abbreviate things by removing vowels.

> Don't be afraid of this rule.  Feel free to invent new "standard" abbreviations, especially for things used a lot (like cow for copy-on-write, ser for serializer, lba for whatever LBA stands for).  For local variables, the rules might be more relaxed -- you might use dc for datacenter, but hopefully not in a context where disk controllers are being used.  Avoid using abbreviations that a first-time reader would not easily understand.  A good sign that an abbreviation is bad is if somebody complains about it.

> See also http://steve-yegge.blogspot.com/2008/09/programmings-dirtiest-little-secret.html

* Run the style checker (in `scripts/check_style.sh`) on your code.

> Sometimes, it even catches bugs!

# Bad Boost Libraries

* Boost Spirit

> It's a horrible, horrible library.

* `boost::lexical_cast`

> The behavior of `boost::lexical_cast<T>(const U &)` for particular values of `T` and `U` is effectively an urban legend. Use a less metaphorical function such as (in replacement of `lexical_cast<int>`) `strtou64_strict`.

* Boost Serialization

> It's slow to compile, overcomplicated, and unhackable.  Use the stuff in `containers/archive` instead.

* Things with C++11 replacements.

> Use `std::function` in place of `boost::function`, and so on, in new code.

# Further Reading

The [Google C++ Style Guide](http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml) is not our style guide, but it is very educational.  It includes some "unwritten" parts of our style guidelines, i.e. things in Sam's head.  (One example: don't use `alloca`.)