This document describes the drawElements coding style for C and C++ languages.
The intention of the drawElements coding guidelines is to allow us to produce code written in a consistent fashion, so that our product line will look similar throughout the line. The guiding philosophy for choosing the described coding style is to avoid bugs when writing code, keep the code maintainable, and also aim to make it beautiful. Some of the decisions are purely a matter of taste, but have been made to keep the code consistent overall (say, camelCasing versus underscore_usage in variable names.
There are also many areas which are not covered by this document and there is some room to bring your own style into the soup. Some of the ways of writing code are just purely matters of opinion. The use of whitespace in code is a good example.
This document is *not* the law of drawElements. If there is a good reason to deviate from it, you should do that. However, if the reason is purely a matter of taste, then please follow the rules set in here. Also, we want to encourage discussion about these guidelines and contributing to them, in case you disagree or know a way of doing something better. This is meant to be an evolving document that follows us as we learn as a group.
A lot of examples are included in this document to make things easily readable and unambiguous. For more source material, feel free to browse the source code of whichever drawElements projects you have visibility to. You should see at least debase and depool libraries, if nothing else.
The main languages at drawElements are Ansi C89 and ISO C++ 98. Ansi C is used for developing driver or middleware IP, while C++ can be used for stand-alone applications.
The reason for using C for middleware IP development is that we build software for mobile devices and the compilers there are often of dubious quality, especially when it comes to support of C++. In addition C++ runtime library is a non-trivial dependency.
Stand-alone userspace applications can be written in ISO C++11.
For utility and tool development, other languages may also be used. So far, Python has been used for all such development and is encouraged to be used in future tools as well. If there are strong reasons, other languages may also be considered.
Let's get started with some sample drawElements code. The code files below show a simple random "class" implemented in C89. The code is taken from the drawElements base portability library, debase.
#ifndef _DERANDOM_H
#define _DERANDOM_H
/*-------------------------------------------------------------------------
* drawElements Base Portability Library
* -------------------------------------
*
* Copyright 2014 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* Id: $Id$
*//*!
* \file
* \brief Random number generation.
*//*--------------------------------------------------------------------*/
#ifndef _DEDEFS_H
# include "deDefs.h"
#endif
DE_BEGIN_EXTERN_C
/*--------------------------------------------------------------------*//*!
* \brief Random number generator.
*
* Uses the Xorshift algorithm for producing pseudo-random numbers. The
* values are generated based on an initial seed and the same seed always
* produces the same sequence of numbers.
*
* See: http://en.wikipedia.org/wiki/Xorshift
*//*--------------------------------------------------------------------*/
typedef struct deRandom_s
{
deUint32 x; /*!< Current random state. */
deUint32 y;
deUint32 z;
deUint32 w;
} deRandom;
void deRandom_init (deRandom* rnd, deUint32 seed);
deUint32 deRandom_getUint32 (deRandom* rnd);
float deRandom_getFloat (deRandom* rnd);
deBool deRandom_getBool (deRandom* rnd);
DE_END_EXTERN_C
#endif /* _DERANDOM_H */
/*------------------------------------------------------------------------- * drawElements Base Portability Library * ------------------------------------- * * Copyright 2014 The Android Open Source Project * \todo insert legalese here. * * Id: $Id$ *//*! * \file * \brief Random number generation. *//*--------------------------------------------------------------------*/ #include "deRandom.h" #include#include DE_BEGIN_EXTERN_C /*--------------------------------------------------------------------*//*! * \brief Initialize a random number generator with a given seed. * \param rnd RNG to initialize. * \param seed Seed value used for random values. *//*--------------------------------------------------------------------*/ void deRandom_init (deRandom* rnd, deUint32 seed) { rnd->x = (deUint32)(-(int)seed ^ 123456789); rnd->y = (deUint32)(362436069 * seed); rnd->z = (deUint32)(521288629 ^ (seed >> 7)); rnd->w = (deUint32)(88675123 ^ (seed << 3)); } /*--------------------------------------------------------------------*//*! * \brief Get a pseudo random uint32. * \param rnd Pointer to RNG. * \return Random uint32 number. *//*--------------------------------------------------------------------*/ deUint32 deRandom_getUint32 (deRandom* rnd) { const deUint32 w = rnd->w; deUint32 t; t = rnd->x ^ (rnd->x << 11); rnd->x = rnd->y; rnd->y = rnd->z; rnd->z = w; rnd->w = w = (w ^ (w >> 19)) ^ (t ^ (t >> 8)); return w; } /*--------------------------------------------------------------------*//*! * \brief Get a pseudo random float in range [0, 1[. * \param rnd Pointer to RNG. * \return Random float number. *//*--------------------------------------------------------------------*/ float deRandom_getFloat (deRandom* rnd) { return (deRandom_getUint32(rnd) & 0xFFFFFFFu) / (float)(0xFFFFFFFu+1); } /*--------------------------------------------------------------------*//*! * \brief Get a pseudo random boolean value (DE_FALSE or DE_TRUE). * \param rnd Pointer to RNG. * \return Random float number. *//*--------------------------------------------------------------------*/ deBool deRandom_getBool (deRandom* rnd) { deUint32 val = deRandom_getUint32(rnd); return ((val & 0xFFFFFF) < 0x800000); } DE_END_EXTERN_C
The following code, taken from deutil demonstrates how C++ classes should look like.
#ifndef _DEUNIQUEPTR_HPP
#define _DEUNIQUEPTR_HPP
/*-------------------------------------------------------------------------
* drawElements C++ Base Library
* -----------------------------
*
* Copyright 2014 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*//*!
* \file
* \brief Unique pointer.
*//*--------------------------------------------------------------------*/
#ifndef _DEDEFS_HPP
# include "deDefs.hpp"
#endif
namespace de
{
/*--------------------------------------------------------------------*//*!
* \brief Unique pointer
*
* UniquePtr is smart pointer that retains sole ownership of a pointer
* and destroys it when UniquePtr is destroyed (for example when UniquePtr
* goes out of scope).
*
* UniquePtr is not copyable or assignable. Pointer ownership cannot be
* transferred between UniquePtr's.
*//*--------------------------------------------------------------------*/
template<typename T, class Deleter = DefaultDeleter<T> >
class UniquePtr
{
public:
explicit UniquePtr (T* const ptr, Deleter deleter = Deleter());
~UniquePtr (void);
T* get (void) const throw() { return m_ptr; } //!< Get stored pointer.
T* operator-> (void) const throw() { return m_ptr; } //!< Get stored pointer.
T& operator* (void) const throw() { return *m_ptr; } //!< De-reference stored pointer.
operator bool (void) const throw() { return !!m_ptr; }
private:
UniquePtr (const UniquePtr<T>& other); // Not allowed!
UniquePtr operator= (const UniquePtr<T>& other); // Not allowed!
T* const m_ptr;
Deleter m_deleter;
};
/*--------------------------------------------------------------------*//*!
* \brief Construct unique pointer.
* \param ptr Pointer to be managed.
*
* Pointer ownership is transferred to the UniquePtr.
*//*--------------------------------------------------------------------*/
template<typename T, class Deleter>
inline UniquePtr<T, Deleter>::UniquePtr (T* const ptr, Deleter deleter)
: m_ptr (ptr)
, m_deleter (deleter)
{
}
template<typename T, class Deleter>
inline UniquePtr<T, Deleter>::~UniquePtr (void)
{
m_deleter(m_ptr);
}
} // de
#endif // _DEUNIQUEPTR_HPP
Each project should have a prefix of its own. For drawElements base libraries, the prefix de is used. Other projects should use a different, arbitrary prefix. For instance, the stitcher project uses the xo prefix.
Anything which has a reasonable possibility of causing a naming conflict should be prefixed. This includes files, structs, enums, functions (except private ones), macros, etc. In C projects, just about everything in the code needs to be prefixed (files, struct, enums, global functions, etc.), but in C++ code, namespaces remove the need for most prefixing. File names and macros should still be prefixed in C++ code as well. Note that members of classes (either C or C++), or structs or unions do not need to be prefixed with the package prefix.
Identifiers are generally typed in camelCase. This applies to file names, structs, enums, local variables, and struct members. In some cases, prefixes are used to clarify the behavior of a variable. Static variables are prefixed with s_, global variables with g_, and C++ class member variables with m_. Macros and enum entries should always be written in UPPER_CASE with underscores separating the words. Members of C classes don't need to be prefixed.
When emulating classes in C, the class name itself should be written in CamelCase, but starting with a upper-case letter. Usually the classes are prefixed: xoArmEmu, deRandom, but if the class only exists within a single .c file, the prefix can be omitted: StringBuilder. The member functions of the class should be prefixed with the full class name and an underscore, followed by a camelCased function name: xoArmEmu_emulateCode().
Examples of correctly named identifiers:
Naming your variables is somewhat of a black art, but the main goal of giving a name should be clarity. You want to communicate what the contents of the variable mean. The more obscure the purpose of a variable is, the longer (and more descriptive) a name you should invent for it. Also, the longer the life time of a variable is, the longer a name it deserves. For example, a loop counter which is alive for page worth of code should be named something like vertexNdx, whereas a loop counter which lives only a couple of lines can be named simply i or ndx.
Most variables should be declared const and never changed (see coding philosophy section). Thus one often successful approach for variable naming is to give name for the value instead. For example when querying first child of node and storing it in variable, that should be named as firstChild instead of node.
Consistency is one important factor in naming variables. When a similar kind of name is needed in multiple places, choose a way of devising the name and stick to that. E.g., if you query the number of elements in an array to a local variable in several functions, always use the same name in each of the functions.
When dealing with counts or numbers (number of elements in an array, etc.), you should always clearly indicate with the name that this is the case, e.g., numElements (preferred), elementCount, etc. Which ever prefix or postfix you choose to use, stick to it.
Function parameters that have an unit of measure (e.g. seconds or bytes) should have the unit as part of the name, for example timeLimitMs and chunkSizeKb.
Use American English instead of English English. Choose gray over grey, color over colour, and so forth.
| buffer | buf |
| destination | dst |
| index | ndx |
| source | src |
| variable | var |
For enums and structs, the types should always be typedeffed and used without the struct or enum prefix in actual code.
/* Declaration. */
typedef enum xoConditionCode_e
{
...
} xoConditionCode;
typedef struct deMempool_s
{
...
} deMemPool;
/* Usage. */
deMemPool* memPool;
xoConditionCode condCode;
All header files should have include guards in them to avoid processing them multiple times in case they are included from multiple places. The style used for the macro is _FILENAME_H, for example: _DEDEFS_H. Whenever including other headers from a header file, you should always use external include guards as well. The external include guards considerably reduce the number of file accesses that the compiler needs to make, resulting in faster compile times.
Each implementation file should have matching header file and vice versa. The implementation file must include the corresponding header file first. By doing that, it is guaranteed that the header file includes all of its dependencies.
Each header file should first include deDefs.h, or alternatively project-specific xxDefs.h/hpp file that in turn includes deDefs.h. That way all the usual types and macros are always properly defined.
#ifndef _DEDEFS_H # include "deDefs.h" #endif #ifndef _DEINT32_H # include "deInt32.h" #endif #ifndef _DEUNIQUEPTR_HPP # include "deUniquePtr.hpp" #endif
The include order of files should start from debase (esp. deDefs.h), go thru other base libraries, then your own project header files, and lastly the system header files. Also, a .c file must include its own header file first. E.g., deMemPool.c must first include deMemPool.h.
Every include path must also end up including deDefs.h before any actual code is processed. This ensures that the basic portability macros (DE_OS, DE_COMPILE, etc.) have been defined.
Code should be indented with tabs (instead of spaces) and a tab-width of 4 characters should be used.
Always put braces on their own lines. This applies to functions, structs, enums, ifs, loops, everything. The only exception are single-line scopes. For one-statement ifs or loops, braces should not be used. Also, put else and else if on their own lines as well.
void main (int argc, const char** argv)
{
if (argc > 1)
parseArgs(argv[1]);
else
{
printf("Usage:\n");
printf("...\n");
}
}
In addition to only indenting your code, things like variable names in a list of declarations or comments at the end of line, should also be aligned such that they start at the same column. Compare the following two examples of the same code, only with differing alignments in the text.
struct deMemPool_s
{
deUint32 flags; /*!< Flags. */
deMemPool* parent; /*!< Pointer to parent (null for root pools). */
deMemPoolUtil* util; /*!< Utilities (callbacks etc.). */
int numChildren; /*!< Number of child pools. */
deMemPool* firstChild; /*!< Pointer to first child pool in linked list. */
deMemPool* prevPool; /*!< Previous pool in parent's linked list. */
deMemPool* nextPool; /*!< Next pool in parent's linked list. */
...
};
struct deMemPool_s
{
deUint32 flags; /*!< Flags. */
deMemPool* parent; /*!< Pointer to parent (null for root pools). */
deMemPoolUtil* util; /*!< Utilities (callbacks etc.). */
int numChildren; /*!< Number of child pools. */
deMemPool* firstChild; /*!< Pointer to first child pool in linked list. */
deMemPool* prevPool; /*!< Previous pool in parent's linked list. */
deMemPool* nextPool; /*!< Next pool in parent's linked list. */
...
};
Always use C-style comments in C code: /* This is a C comment. */ Only use the C++ // end-of-line comments in C++ code.
/* Use this kind of comments in C code. */ // This kind of comments may only be used in C++ code.
// Good: pointers and references are a part of the type void* ptr; deInt32* colorBuffer; xoArmEmu* armEmu; Array<int>& intArray; void doBlend (deUint32* dst, const deUint32* src); // Bad: pointer symbol should not be a part of the name void *ptr; void doBlend (deUint32 *dst, const deUint32 * src);
// Good: void if empty param list, empty space after name, braces on own line
void doStuff (void)
{
}
// Bad: horrible function name!
void doStuff() {
}
// Good: separate arguments with spaces, function name
ShapeList getIntersectingShapes (float x, float y, float z)
{
}
// Bad: function name (list of what volumes?), no space after commas in arg list
ShapeList getShapeList (float x,float y,float z)
{
}
// Exception: sometimes simple function are best written as one-liners
float deFloatAbs (float f) { return (f < 0.0f) ? -f : f; }
// Good: no extra braces for one-liner if cases
if (a.isZero)
result = 0.0f;
else
result = a.value * (1.0 / 65536.0f);
// Bad: extraneous braces, bad whitespace usage
if (a.isZero)
{
result=0.0f;
}
else
{
result=a.value*(1.0 / 65536.0f);
}
// Good: expression easy to read
if (a.isZero && b.isZero)
{
...
}
// Bad: missing spaces around && operator, missing space after 'if'
if(a.isZero&&b.isZero)
{
...
}
// Good: else on its own line
if (alpha == 0)
{
...
}
else if (alpha == 255)
{
...
}
else
{
...
}
// Bad: else on same line as closing brace
if (alpha == 0)
{
...
} else if (...)
{
...
} else
{
...
}
// Good: note space after 'while'
while (numTriangles--)
{
...
}
// Bad: whitespace usage
while(numTriangles --)
{
...
}
// Good: while on same line as closing brace
do
{
...
} while (--numTriangles);
// Bad: while on its own line, missing whitespace after 'while'
do
{
...
}
while(--numTriangles);
// Good: easy to read
for (ndx = 0; ndx < numTriangles; ndx++)
// Bad: missing spaces all over (whitespace should be used to separate expressions)
for(ndx=0;ndx<numTriangles;ndx ++)
// Good: note missing braces for while, correct usage of whitespace
while (numTriangles--)
area += computeArea(triangle[ndx++]);
// Bad: don't put unnecessary braces, avoid extraneous whitespace in expressions
while (numTriangles--)
{
area+=computeArea( triangle [ndx++] );
}
// Good: case-statements indented, code indented another level (including breaks)
switch (blendMode)
{
case XX_BLENDMODE_NORMAL: // no variable declarations
...
break;
case XX_BLENDMODE_SRC_OVER: // need braces if declaring variables inside
{
int alpha = ...;
break;
}
case XX_BLENDMODE_XYZ:
...
// FALLTHRU! -- make non-breaked cases very explicit!
default: // handles the final blendmode (DISABLED) with an assertion!
DE_ASSERT(blendMode == XX_BLENDMODE_DISABLED);
break; // always put break!
}
// Bad:
switch(blendMode)
{
case XX_BLENDMODE_NORMAL: // always indent case labels
...
break; // put break on same level as indented code!
case XX_BLENDMODE_SRC_OVER:
{
...
break;
}
case XX_BLENDMODE_XYZ:
...
case XX_BLENDMODE_DISABLED: // always comment the case fall-through (like above)
...
} // default case missing! always need to handle it (and assert if illegal!)
// Good: parenthesis or whitespace used to indicate evaluation order array[(a * b) + c]; array[a*b + c]; // Bad: order unclear array[a*b+c]; // Good: parenthesis (or whitespace) makes evaluation order unambiguous array[(a && b) || (c == 0)] array[a==0 || b==0 || c==0] // in some cases spaces can be used instead of parenthesis // Bad: unclear evaluation order array[a&&b || c==0] // does this even work? array[a == 0 || b == 0 || c == 0] // Good: easy to see different parts of evaluation (whitespace where it matters) array[triangle->index0 - cache.baseIndex]; // Bad: hard to read (whitespace around brackets doesn't help readability!) array[ triangle->index0-cache.baseIndex ]; array [triangle -> index0 - cache.baseIndex]; // Good: easy to see all function arguments computeArea(vtx0.x, vtx0.y, vtx1.x, vtx1.y, vtx2.x, vtx2.y); // Bad: missing spaces makes it hard to read, no space after function name when calling computeArea ( vtx0.x,vtx0.y,vtx1.x,vtx1.y,vtx2.x,vtx2.y ); // Good: readable (the code itself is a made-up example and thus incomprehensible) // Consider: would probably make more readable code to use temporary variables here if (sizeArray[a+5] > getSize(getFoo()+2)) if (sizeArray[a + 5] > getSize(getFoo() + 2)) // Bad: whitespace usage confuses rather than helps if(sizeArray[a+5]>getSize(getFoo()+2)) if ( sizeArray [ a + 5 ] > getSize ( getFoo () + 2 ) ) // Bad: unclear (and wrong) evaluation order if (bitMask & (1<<bit) == 0)
#if defined(DE_DEBUG) // prefer #if defined() to #ifdef
...
#endif /* DE_DEBUG */ // only put ending comment if #if is far away
TODO: explain all of these
- DE_COMPILER, DE_OS, DE_CPU - basic types (deUint8, deIntptr, deBool==int, ..) - DE_NULL - DE_DEBUG -- #if defined(DE_DEBUG) - DE_INLINE - DE_ASSERT(), DE_VERIFY(), DE_TEST_ASSERT(), DE_STATIC_ASSERT() - DE_BREAKPOINT() - DE_SWAP() - DE_LENGTH_OF_ARRAY() - DE_OFFSET_OF() - DE_UNREF() - DE_BEGIN_EXTERN_C, DE_END_EXTERN_C - DE_NULL_STATEMENT
- deInt32.h: deInRange32(), deInBounds32(), hashing - deFloat16.h: fp16<->fp32 - deMath.h: generic float math - deRandom.h: random number generation - deMemory.h: allocating memory, deMemset(), deMemcpy(), DE_NEW(), DE_DELETE() - deString.h:
- memory pools (deMemPool) - pooled data structures * Array * Set * Hash * HashArray * HashSet
Each source file should contain the following comment box. In header files the comment is placed after the #ifdef-#endif pair. On implementation files the comment box is placed at the beginning.
/*------------------------------------------------------------------------- * Full Module Name * ---------------- * * Copyright 2014 The Android Open Source Project * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. * *//*! * \file * \brief Short description of the contents. * * Followed by longer description if necessary (such as high-level algorithm * description). *//*--------------------------------------------------------------------*/
TODO:
TODO: single-line, multi-line
Below and example of code commenting for C. When doing C++, you can replace C-style comments with C++-comments.
callFoo(&a); /* Comment about following block (Note empty line before and after)*/ callBar(&b); c = a + b; /* Why we need to do this op */ doItAll(a, b, c); /* Badness starts with this comment */ callBar(&b); /* Why we need to do this op */ c = a + b; doItAll(a, b, c);
Todo-comments should use the following syntax:
/* \todo [2012-01-26 pyry] Give a longer description of todo-usage in code. */
If you wish to communicate to fellow developer about some unexpected behavior or corner-case that is not obvious, \note tag can be used.
/* \note Tangent may be zero. */
TODO: explain
When declaring function arguments, local variables, or class members, all non-mutable ones must be declared const. Declaring variable const communicates clearly your intent to not modify the given value. This is especially important in function argument lists.
Declaring local variables, or function arguments that are passed by value, const, may be a bit controversial. There are indeed a lots of existing code that doesn't follow this rule. However, adding extra constness has proven to improve code readability a quite bit and thus all new code must use const correctly. The only exception is function arguments passed by value; for those const keyword can be omitted. By-value function arguments are however considered to be const for all purposes.
// Function example. Note const qualifier on maxDepth as well which is passed by value.
static glu::VarType generateRandomType (const int maxDepth, int& curStructIdx, vector<const StructType*>& structTypesDst, Random& rnd)
{
const bool isStruct = maxDepth > 0 && rnd.getFloat() < 0.2f;
const bool isArray = rnd.getFloat() < 0.3f;
...
}
// Class members
class Node
{
public:
Node (Node* const parent);
~Node (void);
...
private:
Node* const m_parent;
};
Node::Node (Node* const parent)
: m_parent(parent) // Const members can be initialized
{
}
All variables should be declared at the beginning of a block. If variables are introduced in the middle of code, nested block must be used. This is what ANSI C requires, and the same style must be used in C++ code as well. The only exception for this is loop counters in C++; they may be declared in loop init expression.
Having variable declarations always at the beginning of the block makes code easier to read as no new state is introduced in the middle of code. It also guides towards writing smaller functions that don't use too many variables.
static void logTransformFeedbackVaryings (TestLog& log, const glw::Functions& gl, const deUint32 program)
{
int numTfVaryngs = 0;
int maxNameLen = 0;
gl.getProgramiv(program, GL_TRANSFORM_FEEDBACK_VARYINGS, &numTfVaryngs);
gl.getProgramiv(program, GL_TRANSFORM_FEEDBACK_VARYING_MAX_LENGTH, &maxNameLen);
GLU_EXPECT_NO_ERROR(gl.getError(), "Query TF varyings");
{
vector<char> nameBuf(maxNameLen+1);
for (int ndx = 0; ndx < numTfVaryngs; ndx++)
{
...
TODO: minimize life-time of a variable (may sometimes need additional scopes in C)
TODO: assign zero to first, let compiler assign others (in typical lists)
TODO: use ENUM_LAST
TODO: mask values
TODO: use instead of #defines
TODO: typedef xxEnumName_e trick (already explained above?)
There are generally two types of errors that can occur in code; errors that stem from environment or bad input, and errors that are caused by logic error in the code. Former ones are typically outside our control (such as running into a network error) and latter are simply programming mistakes.
External errors must be handled in a graceful way. Depending on the project it may include handling out-of-memory situations as well (most certainly when doing drivers or middleware). In C function return value should be used for communicating whether external error was hit. In C++ code exceptions can be used as well. Assertions must not be used for checking external error conditions.
Internal logic errors must be checked with assertions. See next section.
Assertions are a form of code documentation. They explicitly declare what the code expects from input values or current state. They are tremendously useful when trying to understand how certain piece of code should be used. In addition they are a very nice debugging aid as they help catch logic errors early on before those errors get chance to corrupt program state.
Functions should assert all non-trivial input data and conditions. The one notorious exception is that pointer validity doesn't need to be asserted if the pointer is dereferenced immediately. Non-trivial computation results should also be checked with assertions.
// Examples of good assertions:
void* deMemPool_alignedAlloc (deMemPool* pool, int numBytes, deUint32 alignBytes)
{
void* ptr;
DE_ASSERT(pool); // Must be asserted since not dereferenced but passed to another function
DE_ASSERT(numBytes > 0); // Assertion on input data condition
DE_ASSERT(deIsPowerOfTwo32((int)alignBytes)); // Non-trivial input condition
ptr = deMemPool_allocInternal(pool, numBytes, alignBytes);
DE_ASSERT(deIsAlignedPtr(ptr, alignBytes)); // Assertion on computation result
return ptr;
}
// Badness starts here
void getTextureWidth (const Texture* texture)
{
DE_ASSERT(texture); // Bad: unnecessary, will crash anyway if texture is null
return texture->width;
}
void doStuff (void)
{
int i = 3;
i += 2;
DE_ASSERT(i == 5); // Bad: assertion on trivial computation result
FILE* f = fopen("myfile.txt", "rb");
DE_ASSERT(f); // Bad: there are legitimate reasons for failure
}
TODO: DE_STATIC_ASSERT lookup table size - should usually match to ENUM_TYPE_LAST
typedef enum xxBlendEquation_e
{
XX_BLEND_EQUATION_ADD = 0,
XX_BLEND_EQUATION_SUBTRACT,
XX_BLEND_EQUATION_REVERSE_SUBTRACT,
XX_BLEND_EQUATION_LAST
} xxBlendEquation;
// Note: size is left for compiler to figure out
static const s_blendModeMap[] =
{
GL_FUNC_ADD, // XX_BLEND_EQUATION_ADD
GL_FUNC_SUBTRACT, // XX_BLEND_EQUATION_SUBTRACT
GL_FUNC_REVERSE_SUBTRACT // XX_BLEND_EQUATION_REVERSE_SUBTRACT
};
// This will cause compilation failure lookup table size gets out of date
DE_STATIC_ASSERT(DE_LENGTH_OF_ARRAY(s_blendModeMap) == XX_BLEND_EQUATION_LAST);
TODO: DE_STATIC_ASSERT of struct sizes
TODO: use small datatypes (deUint8 instead of deBool) when size matters.
TODO: avoid too verbose code.
// Good: compact without sacrificing readability
return (a < 0.0f) ? -a : a;
// Bad: waste of space
float result;
if (a < 0.0f)
{
result = -a;
}
else
{
result = a;
}
return result;
TODO: how declaration looks like (already shown in example..)
TODO: function definitions inside class ok if single-line, other special cases
TODO: copy ctor, assignment operator
// Constructors
FooAtom::FooAtom(int proton, float electron)
: m_proton (proton) // Note aligning member initializers.
, m_electron (electron)
{
}
// Remember to add the name of the namespace at the end of the namespace
namespace foo
{
// Namespaces aren't indented
class Proton;
...
} // foo
Everyone should get familiar with RAII. In a nutshell, "resource acquisition is initialization" means that a class destructor must always release all resources (such as memory or OS handles) that have been allocated during the whole lifetime of the object.
RAII is essential for exception-safe code. You should always make sure that if an exception is thrown, including out-of-memory cases, your code behaves properly and releases all allocated resources.
In C++ references should be generally preferred over pointers. The main difference between pointers and references is that references can not change, and are not expected to be null. References should be used instead of pointers for passing objects when both conditions hold; object can not be null nor reference won't be modified once initialized.
Pointers are used when there is need to change the address, or it can be null for a valid reason. Additionally, pointers are always used for passing basic type or object arrays.
TODO: describe stl container usage policies
TODO: exceptions can be used, custom ones must be based on std::exception
TODO: when to use virtual functions, virtual destructor
TODO: namespace naming
TODO: using statement, never using in headers
Git is currently the weapon of choice for source control management. Even though it is not the perfect solution, it gets job done well, or at least better than most other solutions.
Our repositories are hosted on github.com. You are allowed and encouraged to push any number of new branches to the github repositories. Remember to clean up the obsolete ones after they have been merged to master. But never delete a remote branch that hasn't been created by you.
Before you commit anything, make sure user.name and user.email are properly set up.
git config --global user.name "Veijo Elements" git config --global user.email "veijo.elements@drawelements.com"
The standard line ending format for all text files is Unix-style. The best way to handle line endings on Windows systems is to set core.autocrlf to input. That causes conversion to Unix-style line endings on commit only (i.e. not in checkout).
git config --global core.autocrlf input
In order to keep trailing whitespace out of source tree, a standard pre-commit hook must be placed in each local clone of any source repositories.
# in repository directory cp ~/Dropbox/drawElements/Misc/git/pre-commit .git/hooks/
CMake is used as an official project file generator. CMake can be used to generate makefiles or project files for most IDEs. Unless there is a good reason, you should use project files generated by CMake.
You are free to choose any IDE or editor you like. At least Visual Studio, vim and emacs have been successfully used in the past. Good debugger integration is strongly recommended.
Each class should have only a single purpose to fulfill, and it should encapsulate that entirely. All functionality that is secondary and doesn't require access to classes' internal implementation should not be part of that class. This is called single responsibility principle. It is probably easier to grasp it with an example.
Consider a Texture2D class that manages 2D-dimensional texture data. Such class is clearly responsible for managing lifetime of the associated memory, and storing properties such as size and format. Now, one could need a function for blitting (copying) portion of one texture to some position in an another texture. This could be added as a method to texture class, but it most certainly isn't core responsibility of that class. So correct way to implement that is either as a plain function operating on publicly accessible methods of Texture2D class, or as a separate Blitter class. Same applies to things such as reading texture from a file, clearing the texture to a certain color and so forth.
class Texture2D
{
public:
Texture2D (const TextureFormat format, const int width, const int height);
Texture2D (const char* const filename); // Bad: not core functionality
~Texture2D (void);
// Good methods: essential functionality
Vec4 getPixel (const int x, const int y) const;
void setPixel (const int x, const int y, const Vec4& c);
const deUint8* getPixelPtr (void) const;
// Bad: non-essential
void clear (const Vec4& c);
bool containsColor (const Vec4& c) const;
void setInitialized (void); // Why texture would store bit that belongs outside?
private:
// Good: essential, minimum data set
vector<deUint8> m_pixels;
TextureFormat m_format;
int m_width;
int m_height;
// deUint8* m_pixels; // Bad: explicit mem. mgmt, not core functionality
bool m_initialized; // Bad: extraneous information
};
// Good: independent functions operating on textures
void clearTexture (Texture2D& texture, const Vec4& color);
Texture2D* createFromFile (const char* const filename);
One sign of a successful class design is that the interface feels natural to use. Thus when designing a new class from a scratch, you should start by writing the use cases first. Class interface can be refined until it suits the most important use cases, and only then the implementation is filled in. Doing things in reverse order often leads to interfaces that are later found to be inadequate.
When writing the internal implementation a lot of thought should be put on maintaining consistent state, or more formally, class invariant. Member variables in a class are a form of global state and thus special care must be taken when manipulating that state. If class requires a lot of state, it can be helpful to group some of the members into separate state-only classes whose sole responsibility is maintaining the class invariant for that set of members. Another good pattern is to write a state validation function that is called in debug builds after each non-trivial state change.
Only a minimal set of class member variables should ever be used. If some value can be derived with a relatively little effort from the minimal set of members, it must not be stored as a member variable. In the Texture2D class example, length of a pixel row or image size can be derived from size and format and thus member variables must not be used for them.
Pretty much everyone can agree that relying on global state is undesirable. However, what is not always obvious is what counts as a global state. Global variables are clearly such state, but many more can be considered as well. For example state encapsulated in shared objects, state retained in library API, or even state passed in member variables between member functions could be counted as a form global state. Another way to define global state is that it is anything that can be passed from one function to another without including it in function call arguments.
All forms of global state should be used only when necessary. Excluding some very rare cases, mutable global variables are never necessary. Singletons are really just a fancier version of global variables. Instead of using for example singleton for application log object, it should be passed in explicitly to all objects and functions that require logging.
Traditional imperative programming puts emphasis on variables. They are thought of being limited resource, used for storing immediate computation results for brief periods of time. In early C days it was even common to declare variable register in order to communicate the compiler that it should place the variable into a register. Things have changed a lot since then, and it is no longer necessary to limit use of variables for performance reasons.
Functional languages declare variables immutable, i.e. they are not really varying values, but instead named values. This often greatly improves code clarity and correctness, as variables can not change unexpectedly. While imperative languages certainly need some amout of mutability, the concept of immutable values certainly has advantages.
As discussed in variable naming section, you often should name a single value, not some storage slot for arbitrary set of values. In such case it makes a lot of sense to treat that as immutable named value, not mutable varibale. In C and C++ that can be explicitly declared with use of const qualifier.
In general the amount of state that is considered mutable in any given context should be minimized. Understanding code is a much more easier if number of things that can change is small. This also guides code towards natural separation into smaller functions.
Limiting number of mutable variables leads to a more functional programming style, where a lot of computation done in initializer expressions at the beginning of a block. This is not necessarily a bad thing as it requires separating any non-trivial computation into separate functions. Most often we only need the result of such computation anyway, and how the value itself is computed is not important for the problem at hand.
std::vector<Node*> topologicalSortFromRoot (Node* const root)
{
// Returning containers is OK if called functions are local and compiler
// can easily do return value optimization.
const std::vector<Node*> allNodes = collectAllNodesFromRoot(root); // Reduce number of mutables by computing outside
std::map<Node*, int> useCounts = computeUseCounts(allNodes); // Uses allNodes value, mutable
std::vector<Node*> liveSet; // Mutable as well
std::vector<Node*> sortedNodes; // Used as return value - only appended to
// We have multiple mutables here. Invariant is that each node that has zero in useCount
// must be either in liveSet or sortedNodes, but not in both.
for (std::vector<Node*>::iterator nodeIter = allNodes.begin();
nodeIter != allNodes.end();
++nodeIter)
{
// Note that nodeIter is not considered mutable here - instead it is iteration-specific
// immutable value.
if (useCounts[*nodeIter] == 0)
liveSet.push_back(*nodeIter); // liveSet is used as return value here
}
while (!liveSet.empty())
{
Node* const curNode = liveSet.back();
liveSet.pop_back();
sortedNodes.push_back(curNode);
...
}
return sortedNodes;
}
Pure functions have two properties. Firstly, the result depends only on the input values and always produces same output value given same set of input values. Secondly, the function does not cause any observable side effects or changes to global state. For example sin(x) is pure function as it always returns the same value for same argument value and does not cause any side effects.
As much of the code as possible should be kept pure. Moving pure parts of logic and computation into separate functions is recommended. Unit testing those pure functions is then much easier.
Mutating objects passed in counts as a side effect. Instead pure functions must return a completely new value. This may not always be feasible and some functions may need to be impure for performance reasons. One way to work around that while remaining as pure as possible is to use separate output-only argument for output value. Perhaps the most ubiquitous example of such function is memcpy().
// Good: pure function (assuming that it doesn't touch global state) vector<int> findUniqueNumbers (const vector<int>& numbers); // Good: single output-only parameter void findUniqueNumbers (vector<int>& dst, const vector<int>& numbers); // Bad: copying a lot of data for sake of pureness LargeStateObject setStateX (const LargeStateObject& state, const int value); // Bad: manipulates input for no reason void removeDuplicates (vector<string>& words);