Coding Conventions
Here are the coding conventions used (or mostly used) in Graphite. For each "rule", I try to give a justification, else I know that nobody will follow the rules, if you got students and/or kids, you know what I mean :)
1) Names
N.1: class names: MyBeautifulClass
N.2: function names (member or global): my_beautiful_function
N.3: data members: my_data_member_
Exceptions: mathematical objects can be uppercase,
like Point3d P_ ; SparseMatrix M_ ...
N.4: variables, parameters: my_beautiful_variable
Exceptions: mathematical objects can be uppercase,
like Point3d P ; SparseMatrix M ...
N.5: constants, enum values: MY_CONSTANT
N.6: everything is in the namespace OGF (Open Graphics Foundation classes)
N.7: Reference counted classes and smart pointers:
If MyCountedClass inherits Counted, add the following typedef in the header file: typedef SmartPointer<MyCountedClass> MyCountedClass_var ;
N.8: file names: my_beautiful_file.h, my_beautiful_file.cpp
We use the ".cpp" extension to facilitate cross-platform development
(it is understood by all C++ compilers, including MSVC)
(These naming conventions together with nil for NULL and XXX_var for smart pointers (see below) may sound familiar to folks who
ever worked with Gocad)
2) C++
C.1: never use macros
Reasons: - you can always use inline functions or enums instead
- inline functions are type-safe and avoid some stupid
bugs occurring with macros
- it is easier to debug inline functions than macros
Exception: FOR_EACH_VERTEX, FOR_EACH_HALFEDGE and FOR_EACH_FACET
(iterators are too annoying to use, and these three macros
improve the legibility of the code)
C.2: always initialize pointers, either to the address of something, either to nil.
Reasons: an uninitialized pointer p contains a random value, and accessing
to *p does not systematically result in a seg fault. In contrast,
if p == nil, accessing to *p systematically causes a seg fault, which
facilitates debugging. The sooner a bug is detected, the easier it is fixed.
Note: in Graphite, NULL is nil (more sexy, I think ...)
C.3: reset pointers to nil after deleting them
Reasons: the memory manager does not reset the memory to zero. This means
accessing deallocated memory does not systematically result in a seg fault.
Reseting pointers to deallocated memory facilitates tracking accesses to
deallocated memory.
C.4: never use char*, always use std::string instead.
When calling an old-style function f expecting a char*, use f(s.c_str())
Reasons: char* is a frequent source of bugs, such as memory leaks, memory
bugs (double deallocations), buffer overflows.
C.5: never use C arrays
* either use Array1d<>, Array2d<>, Array3d<> instead
(from OGF/basic/containers/arrays.h)
* or use std::vector<>
Reasons: Arrayxd<> has automatic memory management and optional bounds checking.
Exceptions: if it is a fixed size structure. In this case, do not forget to
add assertion checks in the accessors (see rule C.9).
C.6: to construct a string from data, never use sprintf, use an ostringstream instead.
Reasons: better type checking, automatic memory management (no buffer overflow)
Example:
#include <sstream>
...
...
std::ostringstream os ;
os << x << y << z << ... << std::ends ;
std::string s = os.str() ;
C.7: never use sscanf, use an std::istringstream instead
Example:
#include <sstream>
...
...
std::string s = ... ;
std::istringstream is(s) ;
is >> x >> y >> z ;
C.8: never use "\n", use std::endl instead.
Reasons: improves legibility, and has the same behavior on all operating systems
C.9: use ogf_assert(condition) as often as possible.
Reasons: the sooner a bug is detected, the easier it is fixed.
Note: each time you correct a bug, try to add an assert somewhere
that would have helped to detect the bug (i.e. do not waste
time twice on the same type of bug).
C.10: classes with virtual functions should have a virtual destructor.
Reasons: this is needed to call the right destructor when deleting
a pointer to the base class.
C.11: classes that do memory allocation should have
- a copy constructor
- an operator=
- a destructor
Reasons: the default copy ctor and operator= just copy the pointers,
not what they point to. It means the allocated memory is shared between
several instances, and their destructor may deallocate the memory several
times. See also the excellent book by James Coplien "advanced C++ programming".
If you do not want to define a copy ctor and an operator=,
declare them as private, as follows (so that the compiler
issues an error if it tries to call them):
private:
MyClass(const MyClass& rhs) ;
MyClass& operator=(const MyClass& rhs) ;
C.12: basic types should be passed by value
void my_function(double x, double y, double z)
C.13: classes and complex types should be passed by reference
void my_function(MyClass& x, MyClass& y, MyClass& z) Reasons: passing by value would copy the objects on the stack.
C.14: use "const" as often as possible
a "const" parameter cannot be modified
a "const" member function cannot modify "this"
if you do not know *exactly* what "const" means and how to use it,
read Stroustrup's book (see H.1).
Reasons: helps detecting errors "at compile time"
Note: basic types (int, float, char, xxx*) should not be passed
as consts. It would be meaningless, since they are copied on the
stack. "const" parameters should only be object references.
C.15: we do not use exceptions in Graphite
Reasons: when we started the project, they were not well implemented in all compilers,
and were causing big performance penalties. Maybe we could try re-introducing them,
but we keep the initial "without exceptions" design for now.
C.16: to iterate on a std::vector, we use the following code:
std::vector<Something> v = ... ;
for(unsigned int i=0; i<v.size(); i++) {
do_something(v[i]) ;
}
rather than:
std::vector<Something> v = ... ;
for(std::vector<Something>::iterator it = v.begin() ;
it != v.end(); it++) {
do_something(*it) ;
}
Reasons: it is both more legible and less annoying to type !
Note: i is an unsigned int (else it causes a warning)
C.17: data members should never be public, use inline "accessors" member functions instead.
Reasons: if the implementation changes, it is easier to
have just the accessors to modify (rather than all the
client code)
Exception: some small classes that are used internally
(they are not exported in any header file, only
present in .cpp files)
C.18: <this rule was removed>
(the rule was a workaround to overcome the bad 'for' scoping rule
of old compilers)
C.19: members of template classes are all inline (even virtual ones, the compiler will generate out-of-line copies of them automatically)
Reasons: this facilitates the compilation of the project. It
would have been possible to put the implementations in a separate
file, but then it is much more difficult to compile.
3) Graphite system
G.1: Each Graphite library has a common.h and a <libname>_common.cpp file located in the "common" subdirectory.
- Each header file of the library should include the library's common.h file before anything else
- Each source file should include the corresponding header file before anything else
Example:
OGF/cells/map/map.h, in the library cells, starts with
#include <OGF/cells/common/common.h>
OGF/cells/map/map.cpp starts with
#include <OGF/cells/map/map.h>
Reason:
Some libraries maintain extra information (factories, GOM reflection API, ...). Including the
common header before everything else ensures that this extra information is properly initialized
before using any class of the library.
G.2: <this rule was removed, does not longer exists with new GOMGEN>
G.3: Shader class names should match the following pattern: XXX<GrobClassName>Shader2d or XXX<GrobClassName>Shader3d
Examples:
PlainSurfaceShader2d, PlainSurfaceShader3d, GPUProgramsSurfaceShader3d ...
G.4: Tool class names should match the following pattern: <GrobClassName>XXX
Examples:
SurfaceEditAnchors, SurfaceEditPins, SurfaceConnectEdges
G.5: Command class names should match the following pattern: <GrobClassName>XXXCommands
Examples:
SurfaceAttributeCommands, SurfaceMeshCommands, ...
G.6: Grob classes, shaders, tools, commands need to be registered to the system
in the <library_name>/common/<library_name>_common.cpp file
Example: see quick_start/common/quick_start_common.cpp
Reason: this is how Graphite constructs the shader lists, the menus, etc...
G.7: The common.h file of each module should start as follows:
#ifdef WIN32 #ifdef PACKAGE_EXPORTS #define PACKAGE_API __declspec( dllexport ) #else #define PACKAGE_API __declspec( dllimport ) #endif #else #define PACKAGE_API #endif (Replace PACKAGE by the name of your package: ex QUICK_START_API) You also need to add the PACKAGE_API macro each time you want to export: - a class; example: static class QUICK_START_API quick_start_libinit {...} ; - a function; example: extern "C" void QUICK_START_API OGF_quick_start_initialize() {...} ; Reason: this is for portability, this makes it possible to do DLLs under Windows. NB: class with inline functions, and templated class cannot be exported using PACKAGE_API and will return an error.
G.8: Create a file named CMakeLists.txt in the directory of your package
to set dependencies and extra linking information.
See the quick_start/CMakeLists.txt file for example.
4) Code formatting
F.1: no more than 80 columns
Reasons: - it is more printer-friendly
- legibility
F.2: indent size = 4 F.3: braces of a block are positioned as follows:
if(...) {
}
F.4: always open a block after an if(), a for(), a while() or
a repeat() construct
Reasons: - avoids some stupid bugs when adding code to the construct
- legibility
F.5: inline functions are right after their definition
Reasons: putting them at the end of the header file or in a separate
file is too annoying
5) Coding habits
H.1): Be friendly with the other developers:
* Document your classes: Graphite uses the "Javadoc" norm,
which is understood by Doxygen.
The Doxygen tool automatically generates the "Graphite programmer's reference" from the header files.
* Document your code: explain your algorithms, data structures, tips and tricks ...
H.2): Read the latest edition of "Bjarne Stroustrup, the C++ language" from cover to cover every two years
(I'm not kidding, this is what I do ... ooops, to be honest, last time was in 2006 :) )
Reasons: - to be a skilled programmer, you need to know *everything* about the
tools you are using.
- C++ keeps evolving, it is important to stay aware of the evolutions
of the language
H.3): Keep a log of all the bugs you have fixed (what it was and how you fixed it)
Reasons: the second time you find a bug of a given type, instead
of thinking "I already fixed this type of bug two months ago
and I cannot remember what it was", you just have to read your
log to find how to fix it.
H.4) Each time you correct a bug, try to add an assert somewhere
that would have helped to detect the bug (i.e. do not waste
time twice on the same type of bug).
H.5) Use the debugger !
In most cases of "Graphite crash", if you are using assertions and the coding
rules concerning pointers (see section C), you'll just need to see the stack
trace to figure out what happened.
H.6) Legibility and clarity are more important than optimization
If you optimize your code, add comments to explain what you
have done.
Epilogue
If you made it so far, you deserve to know: I got both students and kids, and even with the justifications, none of them follow the "coding conventions" :-) Anyway, with just a little bit of effort, this can save a huge amount of time to everybody.