A common coding style is useful. It makes it easier to communicate, and it makes the code easier to read.
Some of the style choices described in this manual are (at least according to me) very well motivated, other are just arbitrary choices. Sometimes there is no reason to pick one particular style over another, but it is still useful to mandate a specific choice to ensure a consistent style.
The purpose of this style guide is not to cover every possible situation. If something is not covered by this guide it is probably something that doesn't matter that much and you are free to use whatever style you are most accustomed to. If you think something should be in the guide, just bring it up to discussion.
If you see something that obviously does not follow the standard in a source file you should feel free to change it. If you see something that perhaps does not follow the standard, but you are not sure, it is better to leave it.
There is not much point in going over the entire codebase looking for style violations. Such nitpicking is not very productive. Instead, just fix the style violations in code you are working on anyway.
Avoid changing the style back and forth in the same piece of code. If you cannot agree on the style, talk about it instead of having a "cold war" in the codebase.
If you disagree strongly with one of the rules in this style guide, you should propose a change to the rule rather than silently rebel.
Naming is a fundamental part of programming. The ease-of-use and "feeling" of an API depends a lot on having good names.
Furthermore, names are harder to changes than implementations. This is especially true for names that are exported "outside" the executable itself, such as names for script functions and parameters in JSON files. For this reason, some care should be taken when selecting a name.
A name should provide all the necessary information to understand what a function or variable is doing but no more than that. I.e., it should not contain redundant information or information that is easily understood from the context (such as the class name).
// BAD: Name provides too little information char *pstoc(char *); float x; void Image::draw(float, float); // BAD: Name provides too much information char *convert_string_in_pascal_string_format_to_c_string_format(char *); float the_speed; void Image::draw_image_at(float, float); // GOOD: Just the right amount char *pascal_string_to_c(char *s); float speed; void Image::draw_at(float x, float y);
If you cannot come up with a good name for something -- think harder and consult your colleagues. Never use a name that you know is bad.
// BAD: // What is link2? How it is different from link? void link(); void link2(); // These names don't mean anything. Stuff the_thing;
The more visible a variable is (the larger scope it has) the more descriptive its name needs to be, because less information is provided by the context.
Consider the number of players in a network game. In a small local loop, it is fine to call the variable n because there is not much to confuse it with, and the context immediately shows where the variable is coming from.
int n = num_players();
for (int i=0; i<n; ++i)
...
If it is a function in the network class we need to be more verbose, because n could mean any number of things in that context.
int Network::num_players()
If it is a global variable, we must be even more verbose, because we no longer have a Network class context that tells us the variable has something to do with the network:
int _num_players_in_network_game;
Note
Global variables should be avoided. If you really need a global variable, you should hide it behind a function interface (e.g., console_server::get()). This reduces the temptation of misusing the variable.
There are two problems with abbreviations in names:
The general rule is "do not use any abbreviations at all". The only allowed exception is:
Note that the rule against abbreviations only applies to exported symbols. A local variable can very well be called pos or p.
Use lower case characters and underscores where you would put spaces in a normal sentence.
This style is preferred for functions and variables, because it is the most readable one (most similar to ordinary language) and functions and variables are the things we have most of.
Do not use any kind of Hungarian notation when naming variables and functions. Hungarian notation is evil.
It is good to use a different standard for classes than variables, because it means that we can give temporary variables of a class good names:
Circle circle;
If the class was called circle, the variable would have to be called something horrible like a_circle or the_circle or tmp.
Being able to quickly distinguish member variables from local variables is good for readability... and it also allows us to use the most natural syntax for getter and setter methods:
Circle &circle() {return _circle;}
void set_circle(Circle &circle) {_circle = circle;}
A single underscore is used as a prefix, because a prefix with letters in it (like m_ ) makes the code harder to read.
Also, using _ makes the member variables stand out more, since there could be other variables starting with m.
It is good to have #define macro names really standing out, since macros can be devious traps when it comes to understanding the code. (Like when Microsoft redefines GetText to GetTextA.)
This is the most readable syntax, so we prefer this when we don't have any reason to do otherwise.
Enums are types, just as classes and structs and should follow the same naming convention.
Enum values have big scope (either global scope or enclosing class scope). In many cases enum values are used in the same way as #define:s of integer constants and as a user of an API you don't care if a constant is implemented with an enum or with a a macro.
#define ALIGN_LEFT = 0
enum {ALIGN_LEFT = 0};
Since enum values have almost as big scope as macros and in many cases are used in the same way, we use the same naming convention as for macros:
enum Align {ALIGN_LEFT, ALIGN_RIGHT, ALIGN_CENTER};
Note that the prefix ALIGN_ on the enum values is needed, since the enum value scope is big. (I consider the big scope of enums a design mistake in C++.)
Again, this is the most readable format, so we choose that when we don't have a reason to do something else.
The .h files should be put in the same directory as the .cpp files, not in some special "include" directory, for easier navigation between the files.
Getter and setter functions should look like this.
Circle &circle() {return _circle;}
void set_circle(Circle &circle) {_circle = circle;}
The getter is called circle rather than get_circle, since the get_ prefix is superfluous. However, we use a set_ prefix to emphasize that we are changing the state of the object.
There are three bracing styles used:
// Single line
int f() {return 3;}
// Opened on same line
while (true) {
do(stuff);
more(stuff);
}
// New line
int X::f()
{
return 3;
}
The first style is typically used for getter and setter functions in the header file to make the header more compact.
The second style is the default for while loops and for-loops with more than one line.
This third is used for class declarations and function declarations in the cpp file.
Consistent bracing style is not super important, but in general the rule should be that the more that is enclosed by the brace, the more space there should be in the brace.
Instead of
// BAD
while (a)
if (b)
c;
Write
while (a) {
if (b)
c;
}
Only the innermost scope is allowed to omit its braces.
The opening and closing of a brace should preferably fit on the same screen of code to increase readability.
Code that is indented four or five times can be very hard to read. Often such indentation comes from a combination of loops and if-statements:
// BAD
for (i=0; i<parent->num_children(); ++i) {
Child child = parent->child(i);
if (child->is_cat_owner()) {
for (j=0; j<child->num_cats(); ++j) {
Cat cat = child->cat(j);
if (cat->is_grey()) {
...
Using continue to rewrite gives a clearer structure:
for (i=0; i<parent->num_children(); ++i) {
Child child = parent->child(i);
if (!child->is_cat_owner())
continue;
for (j=0; j<child->num_cats(); ++j) {
Cat cat = child->cat(j);
if (!cat->is_grey())
continue;
...
Excessive indentation can also come from error checking:
// BAD
File f = open_file();
if (f.valid()) {
std::string name;
if (f.read(&name)) {
int age;
if (f.read(&age)) {
...
}
}
}
This is one of the few cases where goto can be validly used:
File f = open_file();
if (!f.valid())
goto err;
std::string name;
if (!f.read(&name))
goto err;
int age;
if (!f.read(&age))
goto err;
err:
...
This is the default in Visual Studio and a good compromise between readability and succinctness.
For statements, put a space between keywords and paranthesis, put a space before braces on the same line. Do not put any space before a semicolon.
while (x == true) {
do_stuff();
}
Placement of spaces in expressions is not that important. I generally tend to put a space around every binary operator, but not around unary operators (such as array access, function calls, etc).
z = x * y(7) * (3 + p[3]) - 8;
You can use a more terse or a more loose style if you want to... but make sure that the placement of spaces reflects the evaluation order of the expression. I.e. begin by removing spaces around operators that have a higher order of precedence. This is OK:
z = x*y(7)*(3 + p[3]) - 8;
Because * has higher precedence than - and =. This is confusing and not OK:
// BAD z=x * y(7) * (3+p [3])-8;
By default, the visual studio editor left flushes all preprocessing macros. This is idiotic and makes the code really hard to read, especially when the macros are nested:
// BAD
void f()
{
#ifdef _WIN32
#define RUNNINGS_WINDOWS
#ifdef PRODUCTION
bool print_error_messages = true
#else
bool print_error_messages = false
#endif
#else
bool win32 = false
#endif
Instead, indent your macros just as you would normal C code:
void f()
{
#ifdef _WIN32
#define RUNNINGS_WINDOWS
#ifdef PRODUCTION
bool print_error_messages = true
#else
bool print_error_messages = false
#endif
#else
bool win32 = false
#endif
In visual studio go to Tools > Text Editor > C/C++ > Tabs and change Indenting from Smart to Block to prevent the default indenting.
When the entire file is inside one (or several) namespaces, you should not indent the entire file. Indenting an entire file does not increase readability, it just means you will fit less code on the screen.
Instead, put a comment on the closing brace.
namespace skinny
{
void x();
...
} // namespace skinny
When the namespace declaration does not cover the entire file, but only a screenfull or so, then it can be a good idea to indent it.
A lot of style guides say that lines should never be more than 80 characters long. This is overly restrictive. We all have displays that can show more than 80 characters per line and nobody prints their code anymore.
Never write code like this:
// BAD
int x = the + code +
is + indented +
and + I + dont +
want + to + create
+ long + lines;
Either use less indentation or write longer lines.
Don't go crazy with line lengths, scrolling to see the end of the line is annnoying. Also, make sure not to put very important stuff far to the right where it might be clipped from view.
(I think that line wrapping and associated indentation should be handled automatically by the editor, but alas it is only 2010, let's wait 15 more years.)
// comments are better for comments that you want to leave in the code, because they don't have any nesting problems, it is easy to see what is commented, etc.
/* is useful when you want to quickly disable a piece of code.
Commenting out old bad code with /* ... */ is useful and necessary.
It can be useful to leave the old commented out code in the source file for a while, while you check that the new code does not have any bugs, performance problems, etc. But once you are sure of that you should remove the commented out code from the file.
Having a lot of old, unused, commented out code in the source files makes them harder to read, because you constantly ask yourself why was this commented out, maybe the solution to my problem lies in this commented out code, etc. Source control already keeps a version history, we don't need to keep old code in comments.
Put interface (function and class documentation) in the .h file. This makes it easier to find all the relevant interface documentation for someone browsing the .h files.
A drawback of this is that the .h files will become bigger and harder to grasp, but that is a price we are willing to pay.
Doxygen is a well established standard for interface documentation. It also allows us to easy extract HTML documentation for the engine.
We use the Javadoc-syntax for documentation (///) rather than the QT syntax (//!), because it is easier to type. Use @name to name groups of methods/members.
/// @name Time functions /// Elapsed seconds since the start of the program float time();
Most public methods of classes in external interfaces should be documented. Private methods and private variables can be documented when that increases the understanding of the class.
The main source of information about what the code does should be the code itself. The code is always up-to-date, it doesn't lie and no extra effort is required to maintain it. You should not need to add comments that explain what the code does:
// BAD
/// Returns the speed of the vehicle
float sp() {return _sp;}
// Computes speed from distance and time
s = d / t;
// Check for end of file
if (c == -1)
Instead, write code that is self-explanatory.
float speed() {return _speed;}
speed = distance / time;
if (c == END_OF_FILE_MARKER)
Source code comments should be used as hints to the reader who tries to understand the code. They should point out when the code does something which is a little bit clever or tricky, something that may not be immediately obvious from reading just the code. In complicated algorithms that consist of several steps, they are also useful for identifying the separate steps and giving the user a sense of context.
// Use Duff's device for loop unrolling
// See for example: http://en.wikipedia.org/wiki/Duff's_device
switch (count % 8)
{
case 0: do { *to = *from++;
case 7: *to = *from++;
case 6: *to = *from++;
case 5: *to = *from++;
case 4: *to = *from++;
case 3: *to = *from++;
case 2: *to = *from++;
case 1: *to = *from++;
} while ((count -= 8) > 0);
}
The purpose of comments is to convey information. Avoid big cut-and-paste boilerplate comments in front of classes and functions. Make the comments succint and to the point. There is no point in repeating information in the comment that is already in the function header, like this:
// BAD /// @param p1 a point /// @param p2 another point /// @return The distance between p1 and p2 float distance(const Vector3 &p1, const Vector3 &p2);
Source code comments are not and should not be the only kind of documentation. Source code comments are good for documenting details that are directly related to the code, such as reference documentation for an API.
Aside from detail documentation, systems also need high level documentation. The high level documentation should provide an overview of the system and an entry point for programmers who are new to the system. It should explain the different concepts that are used in the system and how they relate to each other, the goals of system and the different design choices that have been made.
High level documentation should not be put in source code comments. That makes it fragmented and hard to read. Instead, it should be created as an HTML document, where the user can read it as a single continuous text with nice fonts, illustrations, examples, etc.
/// @file /// /// This file contains classes for parsing and generating /// XML files.
There is no need for putting dates, author names, etc in the file comment. You can read that from the subversion repository.
If the file contains only a single class, you typically do not need a file comment, just a comment for the main class.
/// Contains the result of a raycast. /// /// @ingroup Physics class RaycastResult
Every class should have a @ingroup definition since that makes them easier to browse in Doxygen.
If a file contains a lot of classes or other logically separate parts, you can add a divider line between them to increase readability:
// ----------------------------------------------------------------------
/// Cost in going from @a p1 to @a p2. /// @note Cost of going in z is 2 times as expensive. static inline float cost(const Vector3 &p1, const Vector3 &p2)
You don't have to comment every single function in the interface. If the function's meaning is clear from its name, then adding a comment conveys no extra information. I.e... this is pointless:
// BAD /// Returns the speed. float speed();
Do not add a super heavy boilerplate comments to functions with parameters, return values, etc. Such comments tend to contain mostly fluff anyway. They convey no more information than a simple comment and they make it much harder to get an overview of the code.
I.e. avoid fluff pieces like this:
// BAD
/************************************************************
* Name: cost
*
* Description: Returns the cost of going from point p1 to p2.
* Note: Cost of going in z direction is 2 times as expensive.
*
* Parameters:
* p1 - The one point
* p2 - The other point
* Return value:
* The cost of going from p1 to p2.
*************************************************************/
static inline float cost(const Vector3 &p1, const Vector3 &p2)
By Scott Meyer. And also the followup books.
All the code in the engine does not have to be super optimized. Code that only runs once-per-frame has very little impact on a game's performance. Do not spend effort on optimizing that code. Consider what are the heavy-duty number-crunching parts of the code and focus your efforts on them. Use the profiler as a guide to finding the parts of the code that matter.
Be very wary of sacrificing simplicity for code efficiency. Your code will most likely live for a long time and go through several rounds of optimization and debugging. Every time you add complexity you make future optimizations more difficult. Thus, an optimization that makes the code faster today may actually make it slower in the long run by preventing future optimizations. Always strive for the simplest possible code. Only add complexity when it is absolutely necessary.
Be aware that the rules of optimization have changed. Cycle counts matter less. Memory access patterns and parallelization matter more. Write your optimizations so that they touch as little memory as possible and as linearly as possible. Write the code so that it can be parallelized and moved to SPUs. Focus on data layouts and data transforms. Read up on data oriented design.
All current compilers understand the #pragma once directive. And it is a lot easier to read than the standard #ifndef syntax:
// BAD
#ifndef _MY_UNIQUE_HEADER_NAME_H_
#define _MY_UNIQUE_HEADER_NAME_H_
...
#endif
// GOOD
#pragma once
Set up your source control system so that it runs the lint script on every checkin. The lint script removes superfluous spaces at the end of lines, normalizes EOL characters and performs some ohter "code cleanup" tasks.