Batteries C++ Coding Style Guide

Batteries C++ Coding Style Guide

This document describes the coding conventions to be followed in this library. Generally, the coding style Batteries uses is modelled after the Google C++ Style Guide. Generally, we also recommend following the advice in the C++ Core Guidelines, except when they contradict the Google style or this document. In summary, the order of precedence is:

1. This document
2. Google C++ Style Guide
3. C++ Core Guidelines

Some of the major points:

• Avoid Exceptions (see also <batteries/status.hpp> docs)
• Always clang-format the code, using the .clang-format file at the top of the Batteries repo
• Optimize for the reader, not the writer

The rest of this document is a collection of call-outs that the Batteries team has found helpful in onboarding new developers and getting them up to speed on good Batteries C++ style.

Casting and Type Conversions

Prefer explicit constructor calls over static_cast when possible

For example, std::string has an implicit constructor(9) that takes a StringViewLike object (which of course includes std::string_view. Therefore it is possible to write:

DO NOT write:

std::string_view v = get_me_a_string_view();
auto s = static_cast<std::string>(v);  // BAD!

However, this is suboptimal for the following reason: because we prioritize ease-of-reading over ease-of-writing, we care a great deal about the cognitive effort it takes for a reader to mentally parse and understand the meaning of a piece of code. Casts, in general, should be avoided when possible, since they open the door to all kinds of undefined and unsafe behavior. Therefore, when a reader sees a cast, they should immediately start paying closer attention, since the stakes of misunderstanding have just risen significantly. The example above does not really warrant this extra expendature of mental effort, so we prefer writing it without the cast:

Instead, DO write:

auto s = std::string{v};  // GOOD!

or, even simpler:

std::string s{v};  // GOOD!

Class Member Access

To enhance readability, all implicit uses of this within a class should be made explicit. For example:

DO NOT write:

class MyClass {
 public:
  void my_method()
  {
    internal_method();  // BAD!
  }

 private:
  void internal_method() 
  {
    ++counter_;  // BAD!
  }

  int counter_;
};

Instead, DO write:

class MyClass {
 public:
  void my_method()
  {
    this->internal_method();  // GOOD!
  }

 private:
  void internal_method() 
  {
    ++this->counter_;  // GOOD!
  }

  int counter_;
};

Environment Variables

Use batt::getenv_as<T> to access environment variables

This is preferred over calling getenv and std::getenv directly because batt::getenv_as automatically handles type conversion/parsing (and parse errors), and forces the calling code to deal with a variable being undefined or incorrectly formatted by returning an optional type.

DO NOT write:

const char* value = std::getenv("N");
i32 int_value = std::atoi(value);  // BUG!  Possible nullptr value not handled

Instead, DO write:

i32 int_value = batt::getenv_as<i32>("N").value_or(100);
    // ^^^
    // GOOD!  Error cases handled and default value is explicitly specified in a readable way

Error Handling: Status, CHECK, ASSERT, and throw

Prefer CHECK/ASSERT to Status/throw only for problems in the code itself

A good test to apply when trying to decide whether to use a CHECK or ASSERT statement, which will abort the program on failure, is whether the error being detected can be corrected in principle only by a coding change to the program. In other words, if it represents something like bad input, resource exhaustion, data corruptions, etc., then CHECK/ASSERT is the wrong mechanism to use. If however, a certain condition should always be true if the programmer's intent is correctly implemented, then CHECK/ASSERT is the way to go.

Use Status internally within Batteries/project code

• Functions that would otherwise return void should return batt::Status
• Functions that would otherwise return some other type R should return batt::StatusOr<R>
• Declare functions as noexcept when returning batt::Status or batt::StatusOr

Use BATT_THROW_IF_NOT_OK to implement interfaces that use exceptions

The use of some third party libraries together with Batteries in a larger application may dictate writing free functions or class/struct member functions that report errors by throwing. Use batt::StatusException at the boundary layer only, and use macros like BATT_THROW_IF_NOT_OK and BATT_OK_RESULT_OR_THROW wherever relevant.

See <batteries/exception.hpp> for more details.

File Names

All source code files live under batteries/src.

• Source, header, inline/implementation, and test sources should be colocated within the same directory.
• Source files should use the .cpp extension.
• Header files should use the .hpp extension.
• Test files for a given source/header should use the .test.cpp suffix.

For example, if you have a header file: src/some_namespace/myutils.hpp, then you should also have:

• src/some_namespace/myutils.cpp
• src/some_namespace/myutils.test.cpp
• src/some_namespace/myutils.ipp

Integer Scalar Types

Please use the type aliases defined in <batteries/int_types.hpp> instead of their built-in names or the more verbose aliases in the standard header <cstdint>.

DO NOT write:

std::vector<long long> nums;
for (std::size_t i=0; i<100; ++i) {
  nums.push_back(i);
}

Instead, DO write:

#include <batteries/int_types.hpp>

// Don't put this in a header, but for .cpp file it is fine!
//
using namespace batt::int_types;
...
std::vector<i64> nums;
for (usize i=0; i<100; ++i) {
  nums.push_back(i);
}