Coding Style Guide

Version: $Id: codingstyleguide.html,v 1.18 2001/12/11 13:54:01 hans Exp $

This document describes the advised coding style for internal code. When coding external stuff, stick to the external style. The style guide is a dynamic document: if we find that a certain style aspect should be changed (because it's ugly, impractical, strange, inconsistent, etc), it can and should be changed (after we agree on it, of course!).

At this moment we have a lot of legacy code, and it seems unlikely that this will change in the near future. Rewriting everything isn't really an option. Make new code clean, and keep it that way. (Comments & flak to nzc@blender.nl)

We should be writing code that is cross platform. There are other people who do the same, and sometimes they have something usefull to say. Not all advice applies to our situation, so read with care.

Mozilla is a cross-platform browser. They have some tips on doing XP C++.
(Add your hint list here)

Index

1. Introduction
2. Layout 2.1. Tab spacing, tabbing 2.2. Parenthesising 2.3. Multiple statments on one line 2.4. Linewidth 2.5. Writing comments
3. Names 3.1. Keywords and other reserved phrases 3.2. Platform identifiers 3.3. Write function prototypes 3.4. Variable and function names 3.5. Using Blender-libraries names
4. Type usage 4.1. Using booleans 4.2. Type conversions 4.3. Char and uchar types 4.4. Type widths
5. Statements and side-effects 5.1. Statements with side-effects 5.2. Functions with side-effects
6. Compiler/linker usage 6.1. Static code checks 6.2. Using dependencies for experimental code and port discrimination

1. Introduction

Coding styles serve the following purposes:

A uniform way of choosing names, formatting and layout makes code easier to read.
Using a standard style for braces, operators, names etc. helps detect errors e.g in parenthesising, name clashes.
Some elements of C and C++ are ambiguously defined by the standard. Different compilers may give different code. A coding style gives hints on how to avoid these ambiguities.

Per issue, the following points are treated:

What: what problem/issue is addressed?
Who: for who is this applicable?
Why: why is this a problem?
Do: what is the advised style? Sometimes there will be a Don't.
Example: an example illustrating this point.

2. Layout

2.1. Tab spacing, tabbing

What: the distance and type of whitespace used for code indenting
Who: Everybody.
Why: Different tabbing styles can lead to messy code. After three editors have mangled a piece of code, it becomes unreadable. This is about everyone making the same choice: if we would all stick to spaces, it would also work.
Do:
- Use tabs for indentation, not spaces. Set your editor accordingly. The width of tabs can be set to your favourite distance. Using approximately 4 spaces/tab seems fairly common.
- Each nesting level (each opening brace) increases the indent level by 1. Since a function starts with an open-brace, unnested code is at indent level 1. (most coding standards do this already).
- Emacs:
```
(add-hook 'c-mode-hook '(lambda () (c-set-style "bsd")))
(indent-tabs-mode t) 
(tab-width 4)
```
  For vi, joe, Visual Dev Studio, etc: who knows how to? Please add here.

Example: When someone has been editing your tabbed code, but inserted spaces, you might find you routine

if (foo = 2) {
      if (bar = 42) {
            getBar();
            goCrazy();
      }
      getWorkDone();
}

look like

if (foo = 2) {
      if (bar = 42) {
    getBeer();
    goCrazy();
      }
    getWorkDone();
}

2.2. Parenthesising

What: Placement of any parenthesis.
Who: Everybody.
Why: Mixed styles of parenthesis placement can be confusing.
Do: I propose to stick to K&R style
- Open function new line
- Following an if, while, for, case, etc. same line
- Close an if, while, for, case, etc. new line
- Close function new line
- If only a single statement follows a choice-statement, it is acceptable to put them on the same line. Parentheses may not be needed:
```
 if (a < b) c = a; else c = b; 
```
  (which is a stupid example, because you would write
```
 c = (a < b ? a : b) 
```
  here).

Example:

int myFooBar(float x)
{
    int i = 0;
	int k = 0;

    while (i < x) {
        int j = 0;
        while (j < i) { k++; j++; }
        i++;
    }
    return k;
}

2.3. Multiple statments on one line

What: Sometimes you want to put several statements on one line. Try to avoid this.
Who: Everybody.
Why: It makes the code harder to read.
Do:
- Think about what is better: more lines, or wider lines. There is no hard rule here. Just remember that most people would not usually expect multiple statements per line.

Example: Your choice: what reads better?

    col[0] = 0; col[1] = 0; col[2] = 0; col[3] = 0;

    col[0] = 0; 
    col[1] = 0; 
    col[2] = 0; 
    col[3] = 0;

The choice will depend on the context.

2.4. Linewidth

What: Try to keep all lines below 80 characters.
Who: Everybody.
Why: It makes the code harder to read, especially on an unadorned text terminal. When debugging, it is nice to have readable code in the debugger as well.
Do:
- Set the fill-column of your editor to 78 or 79 columns.
- The length of variable names can be a pain sometimes. Don't make them too long.
- If you see you are indenting so much that you cannot comply here, make more functions. A nice rule of thumb is that more than three indentation levels means making one more function. Most people will indent something like 4 spaces/tab.

Example: A small, narrow example.

    if (longVariableName > longTestVariableNameWithFrills) {
        doSomethingComplicatedHere();
        result = (( a + b) / 5) - (dev * rot * glups);
        andMoreStuffAfterwardsThatHelpsMakingNamesLong();
    }

looks ok, but

<----text-width---------------->
    if (longVariableName > longT
estVariableNameWithFrills) {      
        doSomethingComplicatedHe
re();
        result = (( a + b) / 5) 
- (dev * rot * glups);
        andMoreStuffAfterwardsTh
atHelpsMakingNamesLong();
    }

doesn't. Try rewriting this to something like

<----text-width---------------->
     if ( var > testvar) {
         doComplicated();
         res = ( ( a + b) / 5 ) 
               - (dev 
                  * rot 
                  * glups); 
         doMore();
     }

2.5. Writing comments

What: Use /* */ for C code comments. Don't nest comments.
Who: C users.
Why: Not all C compilers accept // as comment symbol. If you // your comments, you'll have extra work porting your stuff. Nested comments may hide code from view temporarily or permanently, while keeping you wondering what the hell happened.
Do:
- Use the right comment style for the right language. Don't commit C code with //-comments.
- Some compilers will actually not complain about having nested comments. If you compiler is of this persuasion, do yourself a favour and disable it.
- If you want to block out larger code chunks, consider using #ifdef 0. This might generate a warning.
Example: -

3. Names

3.1. Keywords and other reserved phrases

What: Some letter combinations are keywords.
Who: C++ users, but especially C users.
Why: The C standard is very braindamaged on how naming can and should be used. A lot of stupid things are allowed. Sometimes, it is legal to use reserved (system)identifiers for your own variable names. The program's behaviour is another thing entirely.
Do:
- Please do avoid the following:
  - str.... is a reserved namespace! So are is..., mem..., to..., wcs... . Never have any external variables start with any of these combinations. You will start linking to functions you never knew existed. (actually, you might not, but this being a reserved namespace should make you careful enough).
  - Having a struct x, int x in parallel is legal. It is also a good recipe for a fortnight of impossible debugging. Be careful about this: the standards allows lots of stupid 'conflicts' here. Use a new name for a new thing.
  - Anything starting with underscores. Sometimes this is allowed, but it is better not to rely on this.
  - There is a list available with all illegal names in C. (Illegal according to ISO, ANSI or both). It is depressingly long.
  - In C++, you may find yourself using '#include <iostream>' and '#include <iostream.h>'. The difference is that the .h variety includes and adds everything to the current namespace. The non-.h variety leaves everything in the std namespace (so you need to prefix with std:: before using).
Example: -

3.2. Platform identifiers

What: Determine on what platform you are compiling
Who: Porters
Why: trivial
Do:
The first one is the preferred one.
#ifdef
- Beos: __BeOS
- MacOSX / Darwin: __APPLE__ __ppc__
- FreeBSD: __FreeBSD__ __i386__ See porting-versions.html
- Irix: __sgi __mips
- Linux: __linux__ __i386__ __sparc__ __alpha__ __powerpc__
- Solaris: __sun __sparc __i386
- Windows: _WIN32
#endif
(on gcc:) You can test for predefined names and paths with 'gcc -v filename.c'
(on gcc:) Doing 'echo foo | gcc -dM -E -' lists the set of equivalent #define-s for compiler and library versions.

3.3. Write function prototypes

What: Determine how and where functions are visible.
Who: Everybody (especially C users).
Why: It should be clear which functions are visible where, without looking at linking order or build order. A declaration serves a a sort of contract, where the builder of a function promises to deliver certain functionality. Users shouldn't have to peek at the implementation details to know what is going on. Implicitly declared functions will always return int, and are assumed to return a value, even if they are later on declared to do this differently. Often you will get a link warning/error, but not always.
Do:
- Write declarations for all functions, especially externally visible ones.
Example: -

3.4. Variable and function names

What: How to choose variable and function names.
Who: Everybody.
Why: A uniform naming strategy helps when you are studying a new piece of code. It also frees you from the task of having to come up with yet another stupid name.
Do:
- Choose sensible, descriptive names. Even though the C standard specifies only the first 6 characters have to be used for linkage, this is not true for any modern compiler (notably: Gcc, Visual C++ and the Irix C compiler). Do not feel restricted in name-length (but don't overdo it, of course).
- A member variable from a class is prefixed with m_. Do not prefix other variables this way.
- We don't do extensions to indicate types and such. So don't make iInteger, CClass, bBoolean and so forth.
- Externally visible names must comply to the following rules. It is a good idea to also make internal names comply:
  - c/c++: Function names always start with lower caps.
  - c++: Member names always start with a capital.
  - c/c++: Words within a variable name are separated with underscores, and are all lower caps. This is more readable than the Java-style with caps for each new word. Function-local variables: Pick whatever you find convenient. Try to stick to lower caps, try to keep the names short. Local names may never clash with global names. If this happens, this is a mistake in the implementation of the local name. External names are designed be be easily avoidable.
Example:
- Member variable for counting flutzpahs: m_flutzpah_count
- Method for getting the flutzpah count: GetFlutzpahCount

3.5. Using Blender-libraries names

What: Use prefixes to mark functions and variables in external interfaces
Who: Everybody.
Why: Avoid name clashes, make interface usage clear, make dependencies on other modules and libraries explicit.
Do: At this moment, the following prefixes are in use:
- GEN: General stuff (c++)
- SM: SuMo (Solid Uses Moto), the physics engine (c++)
- KX: Ketsji, the game engine (c++)
- MT: MaTh, mathematics module (c++)
- RE: Render, static rendering (c)
- PR: Portable Runtime, a Netscape library (external code, c)
- PRB: PR-Blender, our own extensions to PR (c)
- SND: SouND, our (currently OpenAL) wrapper and extensions (c)
- RAS: RASterizer, our low level 3D polygon abstraction layer (c++)
- NET: NETworking. Currently our Terraplay CNI wrapper.
- BLI: Blender low-level LIbrary.
- BKE: Blender KErnel.
- BIF: Blender Interface Framework.
- BSE: Blender Space Editor.
- BDR: Blender DRawing.
- DNA: struct DNA, the types definitions of serializable data, and the construction of DNA.o, which encodes this data.
- AVI: conversion of AVI format files.
- IMB: IMage Buffer, all kind of operations on images.
Not everything is a module yet... But we should work on getting there.
Example: Function
```
 pickFlowers() 
```
in the external interface of the GEN module should be called
```
 GEN_pickFlowers() 
```
.

4. Type usage

4.1. Using booleans

What: Use of boolean variables
Who: C users (but C++ users may want to do this as well)
Why: C does not have a native boolean type. Often bitflags can be/are used.
Do:
- Bitflags are used a lot in Blender. This is the advised way of dealing with booleans. Take care when operating on these values. Remember that the compiler is much better at optimizing low-level operations on bits than you are. What is efficient also depends on the platform you're working on.
Don't:
- Never test for something like
```
 myBool == TRUE 
```
  as this is an integer comparison. Unless TRUE happens to be defined as exactly the same value as what the compiler uses internally as TRUE representation, this fails.
```
 !(!TRUE) == TRUE 
```
  is in general false! If you see something like
```
 if (testVal == True) { ... }
```
  you have found a mistake. This should be
```
 if (testVal)
{...} 
```
  which is safe, and more readable.
- Don't make your own TRUE or FALSE definitions.
Example: When you test for a bitflag, you may write something like
```
 
    if ((mask & flagarray) == TRUE) {
```
This goes spectacularly wrong. Stick to
```
 
    if (mask & flagarray) {
```

4.2. Type conversions

What: Type equality, type conversion, casting.
Who: Everybody.
Why: The C standard says: 'Two types are equal if they are the same.' It also says: 'Two type do not need to be the same to be equal'. A lot is determined by the compiler builders.
Do:
- Be consistent with types. Cast carefully. Remember we run on both big and little endian systems.
- We still need to build in a consistent system to handle matrices, matrix calculations and such in the old C code. Try to stick to existing types whereever possible.
Example: -

4.3. Char and uchar types

What: Use of char and uchar.
Who: Everybody.
Why: The choice for char/uchar can sometimes be confusing, since some systems use 8 bit chars, whereas others require uchar to get an 8-bit type.
Do:
- Always use char, never uchar. Compiler flags are used to make sure we always use the unsigned variety.
- gcc: use -funsigned-char
- Visual c++: /J (I think...)
- Irix: does it by default (-signed to disable that)
Example: -

4.4. Type widths

What: Width of types.
Who: Everybody.
Why: Be carefull about the width of types. Converting to a narrower type will lose information.
Do:
- As a reminder, these are the type widths:
  - char by default unsigned (see also previous point), 8 bits
  - short signed integer, 15 bits plus sign
  - int signed integer, 31 bits plus sign
  - long signed integer, depending on the platform, 31 or 63 bits plus sign. Use this when you need to store a pointer in an integer or need to calculate with pointers.
  - long long signed integer, 63 bits plus sign. ANSI-C does not support this by default, but fortunately, all our compilers do.
Example: -

5. Statements and side-effects

5.1. Statements with side-effects

What: Don't let assignments or function calls have (un)expected side-effects.
Who: Everybody.
Why: Any effect you do not expect will make a program unpredictable. And any effect you do expect, someone else might not. Or your expectation might simply be wrong.
Don't:
- Don't let functions do stuff other than what the name suggests. Sometimes this is hard because you want to squeeze extra efficiency out of your code. The least you should do is place some comment that alerts other people to the side effects. The thing is that 'other people' include yourself after 6 months.
- The ubiquitous
```
 if (a = bla()) {...} 
```
  is predictable and short. It also generates warnings because it is easily confused with
```
 if (a == bla()) {...} 
```
  . Avoid the former, just type
```
 a = bla(); if (a) {...} 
```
  .
- When you do
```
 if (bla() && blo() && bli()) 
```
  there is no guarantee that blo() and bli() are not executed when bla() returns true. Some compilers just don't get this right.
- Even more hairy is stuff like
```
 a = f(i++) + i 
```
  or
```
 a = f(i++ + g(i++)) 
```
  or ... Don't do this to yourself. No power on earth can predict what will happen.
Example: -

5.2. Functions with side-effects

What: Don't let functions have unexpected side-effects.
Who: Everybody.
Why: If functions have unexpected side effects, they make the code hard to understand and debug. At the very least, you should document the side effects. Things such as modifying global variables, modifying devices, doing weird stuff with memory etc. count as side effects.
Don't:
- Don't let external functions from modules have side effects that are not well documented.
Do:
- Make the effects functions have as explicit as possible. If a lot of data is modified as a result of a function, this should be documented, or indicated in the function signature.
Example: -

6. Compiler/linker usage

6.1. Static code checks

What: Compilers generate warnings in additions to errors. Warnings can signify errors and mistakes.
Who: Everybody.
Why: While a warning will generally not prevent you from continuing compilation and linking, they are still mistakes or ambiguities in the code that need inspection. A compiler can be configured to emit certain warnings, or to withhold others. Static code checking lint-style is something we do not do here yet.
Don't:
- Don't ignore warnings. Inspect them, and fix them. New code must be written warning-free.
Do:
- Use the following settings for compiling with gcc:
  - -Wall, -W
  - -Wshadow
  - -Wpointer-arith
  - -Wbad-function-cast
  - -Wcast-qual
  - -Wcast-align
  - -Waggregate-return
  - -Wstrict-prototypes
  - -Wmissing-prototypes
  - -Wmissing-declarations
  - -Wredundant-decls
  - -Wnested-externs
  Check the GCC manual for their meanings. In most cases, these warnings will be activated in the Makefile. The warnings flag bad prototyping, bad casting, superfluous variables and such. Warnings can always be avoided: you don't need to leave code that generates warnings because `there's just no other way'. There is. VC displays different warnings. The projectfiles define these, or alternately, the Cygwin wrapper for VC translates the above into the VC equivalent. VC warnings can be disabled by using #pragma's, but this is not adivised. Avoid doing that.
Example: -

6.2. Using dependencies for experimental code and port discrimination

What: Enabling and disabling (experimental) code.
Who: People working on experimental code pieces, or code that does only apply for a limited set of target platforms
Why: Switching off and on of the code must be possible without recompiling everything. Only undefining an environment variable *and* modifying one dependency header file is required.
Don't: Do not only use -DNAN_SOMETHING, this is not enough for c/c++ code as the change of the -D is not seen by make as a reason to recompile.
Do:
- Create one dependency header file per code piece with only '#define NAN_SOMETHING' or '#undef NAN_SOMETHING' in it. This file remains as long as the code piece is under development. All c/c++ files that use '#ifdef NAN_SOMETHING ... #endif' parts need to include the dependency header file.
  For port discrimination in the dependency header file use '#ifdef ... #endif' from the Platform identifiers to guard the NAN_SOMETHING.
- For the GNU make files: Create one environment variable NAN_SOMETHING so that the makefiles can traverse in the code piece directories when the define is set.
Example:
- For the Networking code under development in gameengine/Network (and in gameengine/Ketsji/KXNetwork, gameengine/BlenderRoutines, gameengine/Converter and in blender/src) a single dependency header file gameengine/Network/NG_DependKludge.h was made.
- For the GNU make files (like gameengine/Makefile) a NAN_NET environment variable is used to traverse into gameengine/Network when set.