| <html> |
| |
| <head> |
| <title>drawElements Coding Guidelines</title> |
| |
| <style type="text/css"> |
| div.body { |
| width: 800px; |
| margin-top: 50px; |
| margin-left: auto; |
| margin-right: auto; |
| border: 1px solid silver; |
| background-color: #eee; |
| } |
| |
| div.title { |
| text-align: center; |
| font-size: 24pt; |
| margin-top: 1.5em; |
| margin-bottom: 0.5em; |
| } |
| |
| div.quote { |
| font-style: italic; |
| text-align: center; |
| width: 48%; |
| margin-left: auto; |
| margin-right: auto; |
| } |
| |
| div.copyright { |
| font-style: italic; |
| text-align: center; |
| margin-top: 3em; |
| margin-left: auto; |
| margin-right: auto; |
| } |
| |
| div.author { |
| font-style: italic; |
| text-align: center; |
| margin-bottom: 2em; |
| margin-left: auto; |
| margin-right: auto; |
| } |
| |
| /* All heading elements */ |
| ol > li > .heading { |
| font-family: arial; |
| } |
| |
| /* Heading 1 elements */ |
| ol.h1 { |
| font-size: 15pt; |
| margin-top: 1em; |
| padding-left: 1em; |
| list-style: upper-roman; |
| list-style-position: inside; |
| } |
| |
| ol.h1 > li { |
| margin-top: 2.0em; |
| } |
| |
| ol.h1 > li > .heading { |
| } |
| |
| /* Heading 2 elements */ |
| ol.h2 { |
| font-size: 13pt; |
| margin-top: 1.0em; |
| margin-bottom: 0.5em; |
| |
| padding-left: 1em; |
| } |
| |
| ol.h2 > li { |
| margin-top: 1.25em; |
| } |
| |
| ol.h2 > li > .heading { |
| |
| } |
| |
| ul { |
| margin-bottom: 0.5em; |
| } |
| |
| p { |
| font-size: 12pt; |
| margin: 0.6em; |
| margin-left: 1.3em; |
| border: 0px; |
| } |
| |
| table { |
| font-size: 12pt; |
| margin: 0.6em; |
| margin-left: 1.6em; |
| border: 0px; |
| } |
| |
| table td { |
| padding-right: 10px; |
| } |
| |
| .prettyprint { |
| font-size: 10pt; |
| margin: 0px; |
| margin-left: 2.0em; |
| margin-bottom: 1.0em; |
| padding: 0.1em; |
| padding-left: 0.2em; |
| border: 1px solid black; |
| background-color: #ddd; |
| width: 93%; |
| } |
| |
| .codeTitle { |
| font-style: italic; |
| font-size: 11pt; |
| margin-top: 0.5em; |
| margin-left: 2.0em; |
| margin-bottom: 0px; |
| } |
| |
| </style> |
| |
| <!-- \todo embed --> |
| <link href="prettify.css" type="text/css" rel="stylesheet" /> |
| <script type="text/javascript" src="prettify.js"></script> |
| |
| </head> |
| |
| <body onLoad="prettyPrint()"> |
| |
| <div class="body"> |
| |
| <div class="title">drawElements Coding Guidelines</div> |
| <hr width="50%" /> |
| <div class="quote">"Always code as if the person who will maintain your code is a maniac serial killer that knows where you live."</div> |
| |
| <div class="copyright">Copyright © 2014 The Android Open Source Project</div> |
| |
| <ol class="h1"> |
| <li><span class="heading">Table of Contents</span> |
| <ol class="h2"> |
| TODO: fill in, with links (use JavaScript?) |
| </ol> |
| </li> |
| |
| <li><span class="heading">Introduction</span> |
| <ol class="h2"> |
| <li><span class="heading">Goal and philosophy</span> |
| <p>This document describes the drawElements coding style for C and C++ languages.</p> |
| |
| <p>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.</p> |
| |
| <p>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.</p> |
| |
| <p>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.</p> |
| |
| <p>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 <i>debase</i> and <i>depool</i> libraries, if nothing |
| else.</p> |
| </li> |
| |
| <li><span class="heading">Languages of choice</span> |
| <p>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.</p> |
| |
| <p>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.</p> |
| |
| <p>Stand-alone userspace applications can be written in ISO C++11.</p> |
| |
| <p>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.</p> |
| </li> |
| |
| <li><span class="heading">C code example</span> |
| |
| <p>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.</p> |
| <div class="codeTitle">deRandom.h: The header file.</div> |
| <pre class="prettyprint"> |
| #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 */ |
| </pre> |
| <div class="codeTitle">deRandom.c: The implementation file.</div> |
| <pre class="prettyprint"> |
| /*------------------------------------------------------------------------- |
| * 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 <float.h> |
| #include <math.h> |
| |
| 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 |
| </pre> |
| </li> |
| <li><span class="heading">C++ code example</span> |
| |
| <p>The following code, taken from deutil demonstrates how C++ classes should look like.</p> |
| <div class="codeTitle">deUniquePtr.hpp: Unique pointer template.</div> |
| <pre class="prettyprint"> |
| #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 |
| </pre> |
| </li> |
| </ol> |
| </li> |
| |
| <li><span class="heading">Naming conventions and formatting</span> |
| <ol class="h2"> |
| <li><span class="heading">Basic naming conventions</span> |
| <p>Each project should have a prefix of its own. For drawElements base libraries, |
| the prefix <i>de</i> is used. Other projects should use a different, arbitrary prefix. |
| For instance, the stitcher project uses the <i>xo</i> prefix.</p> |
| |
| <p>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.</p> |
| |
| <p>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 <i>s_</i>, global variables |
| with <i>g_</i>, and C++ class member variables with <i>m_</i>. 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.</p> |
| |
| <p>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: <i>xoArmEmu</i>, |
| <i>deRandom</i>, but if the class only exists within a single .c file, the prefix can be |
| omitted: <i>StringBuilder</i>. The member functions of the class should be prefixed with |
| the full class name and an underscore, followed by a camelCased function name: |
| <i>xoArmEmu_emulateCode().</i></p> |
| |
| <p>Examples of correctly named identifiers:</p> |
| <ul> |
| <li><i>dePool.c, dePool.h, deUniquePtr.hpp, deThread.cpp</i> -- file names</li> |
| <li><i>deRandom, xoStitcher</i> -- structs / classes</li> |
| <li><i>deMemPoolFlag, xoConditionCode</i> -- enums</li> |
| <li><i>DE_COMPILER_MSC</i> -- macros</li> |
| <li><i>XO_BACKEND_NEON</i> -- enum entry</li> |
| <li><i>setTableSize()</i> -- local (static) function</li> |
| <li><i>xoArmEmu_emulateCode()</i> -- C class member function</li> |
| <li><i>numVariables</i> -- local variable</li> |
| <li><i>m_itemHash</i> -- member variable in a C++ class</li> |
| <li><i>s_rcpTable</i> -- static variable in a function</li> |
| <li><i>g_debugFlag</i> -- global variable</li> |
| </ul> |
| </li> |
| |
| <li><span class="heading">Choosing good names</span> |
| <p>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 <i>vertexNdx</i>, |
| whereas a loop counter which lives only a couple of lines can be named simply <i>i</i> or <i>ndx</i>.</p> |
| |
| <p>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 <i>firstChild</i> instead of <i>node</i>.</p> |
| |
| <p>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.</p> |
| |
| <p>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., <i>numElements</i> (preferred), |
| <i>elementCount</i>, etc. Which ever prefix or postfix you choose to use, stick to it.</p> |
| |
| <p>Function parameters that have an unit of measure (e.g. seconds or bytes) should have the unit |
| as part of the name, for example <i>timeLimitMs</i> and <i>chunkSizeKb</i>.</p> |
| |
| <p>Use American English instead of English English. Choose gray over grey, color over colour, |
| and so forth.</p> |
| </li> |
| <li><span class="heading">Canonical abbreviations</span> |
| <table border="0" cellspacing="0"> |
| <tr><td>buffer </td> <td>buf</td></tr> |
| <tr><td>destination </td> <td>dst</td></tr> |
| <tr><td>index </td> <td>ndx</td></tr> |
| <tr><td>source </td> <td>src</td></tr> |
| <tr><td>variable </td> <td>var</td></tr> |
| </table> |
| </li> |
| |
| <li><span class="heading">Struct and enum typedeffing</span> |
| <p>For enums and structs, the types should always be typedeffed and used without the struct or |
| enum prefix in actual code.</p> |
| |
| <div class="codeTitle">Example.</div> |
| <pre class="prettyprint"> |
| /* Declaration. */ |
| typedef enum xoConditionCode_e |
| { |
| ... |
| } xoConditionCode; |
| |
| typedef struct deMempool_s |
| { |
| ... |
| } deMemPool; |
| |
| /* Usage. */ |
| deMemPool* memPool; |
| xoConditionCode condCode; |
| </pre> |
| </li> |
| |
| <li><span class="heading">Header files and including</span> |
| <p>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 <i>_FILENAME_H</i>, |
| for example: <i>_DEDEFS_H</i>. 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.</p> |
| |
| <p>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.</p> |
| |
| <p>Each header file should first include <i>deDefs.h</i>, or alternatively project-specific |
| <i>xxDefs.h/hpp</i> file that in turn includes deDefs.h. That way all the usual types and macros |
| are always properly defined.</p> |
| |
| <div class="codeTitle">External include guard example.</div> |
| <pre class="prettyprint"> |
| #ifndef _DEDEFS_H |
| # include "deDefs.h" |
| #endif |
| #ifndef _DEINT32_H |
| # include "deInt32.h" |
| #endif |
| #ifndef _DEUNIQUEPTR_HPP |
| # include "deUniquePtr.hpp" |
| #endif |
| </pre> |
| |
| <p>The include order of files should start from <i>debase</i> (esp. <i>deDefs.h</i>), go thru |
| other base libraries, then your own project header files, and lastly the system header files. |
| Also, a <i>.c</i> file must include its own header file first. E.g., <i>deMemPool.c</i> must |
| first include <i>deMemPool.h</i>.</p> |
| |
| <p>Every include path must also end up including <i>deDefs.h</i> before any actual code is processed. |
| This ensures that the basic portability macros (<i>DE_OS</i>, <i>DE_COMPILE</i>, etc.) have been |
| defined.</p> |
| </li> |
| |
| <li><span class="heading">Indenting and whitespace</span> |
| <p>Code should be indented with tabs (instead of spaces) and a tab-width of 4 characters should |
| be used.</p> |
| |
| <p>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 <i>else</i> and <i>else if</i> on their own lines as well.</p> |
| |
| <div class="codeTitle">Brace usage</div> |
| <pre class="prettyprint"> |
| void main (int argc, const char** argv) |
| { |
| if (argc > 1) |
| parseArgs(argv[1]); |
| else |
| { |
| printf("Usage:\n"); |
| printf("...\n"); |
| } |
| } |
| </pre> |
| |
| <p>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.</p> |
| |
| <div class="codeTitle">Aligned variable declarations and comments.</div> |
| <pre class="prettyprint"> |
| 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. */ |
| ... |
| }; |
| </pre> |
| |
| <div class="codeTitle">No alignments used.</div> |
| <pre class="prettyprint"> |
| 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. */ |
| ... |
| }; |
| </pre> |
| </li> |
| |
| <li><span class="heading">Other formatting</span> |
| |
| <p>Always use C-style comments in C code: /* This is a C comment. */ Only use |
| the C++ // end-of-line comments in C++ code.</p> |
| |
| <div class="codeTitle">Comment styles.</div> |
| <pre class="prettyprint"> |
| /* Use this kind of comments in C code. */ |
| |
| // This kind of comments may only be used in C++ code. |
| </pre> |
| |
| <div class="codeTitle">Pointer and references.</div> |
| <pre class="prettyprint"> |
| // 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); |
| </pre> |
| |
| <div class="codeTitle">Formatting of function declarations.</div> |
| <pre class="prettyprint"> |
| // 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; } |
| |
| </pre> |
| |
| <div class="codeTitle">Formatting of control statements.</div> |
| <pre class="prettyprint"> |
| // 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++] ); |
| } |
| </pre> |
| |
| <div class="codeTitle">Formatting switch cases.</div> |
| <pre class="prettyprint"> |
| // 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!) |
| </pre> |
| |
| <div class="codeTitle">Formatting of expressions.</div> |
| <pre class="prettyprint"> |
| // 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) |
| </pre> |
| |
| <div class="codeTitle">Other formatting.</div> |
| <pre class="prettyprint"> |
| #if defined(DE_DEBUG) // prefer #if defined() to #ifdef |
| ... |
| #endif /* DE_DEBUG */ // only put ending comment if #if is far away |
| |
| </pre> |
| </li> |
| </ol> |
| </li> |
| |
| <li><span class="heading">Base library services</span> |
| <p>TODO: explain all of these</p> |
| |
| <ol class="h2"> |
| <li><span class="heading"><b>debase</b>/deDefs.h</span> |
| <pre> |
| - 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</pre> |
| </li> |
| |
| <li><span class="heading">Other <b>debase</b> headers</span> |
| <pre> |
| - 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:</pre> |
| </li> |
| |
| <li><span class="heading"><b>depool</b> services</span> |
| <pre> |
| - memory pools (deMemPool) |
| - pooled data structures |
| * Array |
| * Set |
| * Hash |
| * HashArray |
| * HashSet</pre> |
| </li> |
| </ol> |
| </li> |
| |
| <li><span class="heading">Commenting code</span> |
| <ol class="h2"> |
| <li><span class="heading">File comment boxes</span> |
| <p>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.</p> |
| <pre class="prettyprint"> |
| /*------------------------------------------------------------------------- |
| * 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). |
| *//*--------------------------------------------------------------------*/ |
| <pre> |
| </li> |
| |
| <li><span class="heading">Structs/classes/enums comment boxes</span> |
| <p>TODO: </p> |
| </li> |
| |
| <li><span class="heading">Other Doxygen comment boxes (/** ... */ and /*!< ... */)</span> |
| <p>TODO: single-line, multi-line</p> |
| </li> |
| |
| <li><span class="heading">Code comments</span> |
| <p>Below and example of code commenting for C. When doing C++, you can replace C-style comments with C++-comments.</p> |
| <pre class="prettyprint"> |
| 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); |
| |
| </pre> |
| </li> |
| |
| <li><span class="heading">Tags</span> |
| <p>Todo-comments should use the following syntax:</p> |
| <pre class="prettyprint"> |
| /* \todo [2012-01-26 pyry] Give a longer description of todo-usage in code. */ |
| </pre> |
| <p>If you wish to communicate to fellow developer about some unexpected behavior or corner-case |
| that is not obvious, <i>\note</i> tag can be used.</p> |
| <pre class="prettyprint"> |
| /* \note Tangent may be zero. */ |
| </pre> |
| </li> |
| </ol> |
| </li> |
| |
| <li><span class="heading">Generic programming</span> |
| <ol class="h2"> |
| <li><span class="heading">Classes in C</span> |
| <p>TODO: explain</p> |
| </li> |
| |
| <li><span class="heading">Const correctness</span> |
| <p>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.</p> |
| |
| <p>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.</p> |
| |
| <div class="codeTitle">Example.</div> |
| <pre class="prettyprint"> |
| // 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 |
| { |
| } |
| </pre> |
| </li> |
| |
| <li><span class="heading">Declaring variables</span> |
| <p>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.</p> |
| |
| <p>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.</p> |
| |
| <div class="codeTitle">Example.</div> |
| <pre class="prettyprint"> |
| 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++) |
| { |
| ... |
| </pre> |
| </li> |
| |
| <li><span class="heading">Variable life-time</span> |
| <p>TODO: minimize life-time of a variable (may sometimes need additional scopes in C)</p> |
| </li> |
| |
| <li><span class="heading">Enumerations</span> |
| <p>TODO: assign zero to first, let compiler assign others (in typical lists)</p> |
| <p>TODO: use ENUM_LAST</p> |
| <p>TODO: mask values</p> |
| <p>TODO: use instead of #defines</p> |
| <p>TODO: typedef xxEnumName_e trick (already explained above?)</p> |
| </li> |
| |
| <li><span class="heading">Error handling</span> |
| <p>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.</p> |
| |
| <p>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.</p> |
| |
| <p>Internal logic errors must be checked with assertions. See next section.</p> |
| </li> |
| |
| <li><span class="heading">Assertions</span> |
| <p>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.</p> |
| |
| <p>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.</p> |
| |
| <div class="codeTitle">Example.</div> |
| <pre class="prettyprint"> |
| // 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 |
| } |
| </pre> |
| |
| </li> |
| |
| <li><span class="heading">Lookup tables</span> |
| <p>TODO: DE_STATIC_ASSERT lookup table size - should usually match to ENUM_TYPE_LAST</p> |
| |
| <pre class="prettyprint"> |
| 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); |
| </pre> |
| </li> |
| |
| <li><span class="heading">Struct size</span> |
| <p>TODO: DE_STATIC_ASSERT of struct sizes</p> |
| <p>TODO: use small datatypes (deUint8 instead of deBool) when size matters.</p> |
| </li> |
| |
| <li><span class="heading">Extraneous code</span> |
| <p>TODO: avoid too verbose code.</p> |
| |
| <div class="codeTitle">Example.</div> |
| <pre class="prettyprint"> |
| // 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; |
| </pre> |
| |
| </li> |
| </ol> |
| </li> |
| |
| <li><span class="heading">C++ topics</span> |
| <ol class="h2"> |
| <li><span class="heading">Class declarations</span> |
| <p>TODO: how declaration looks like (already shown in example..)</p> |
| <p>TODO: function definitions inside class ok if single-line, other special cases</p> |
| </li> |
| |
| <li><span class="heading">Class boilerplate</span> |
| <p>TODO: copy ctor, assignment operator</p> |
| </li> |
| |
| <li><span class="heading">Code Formatting</span> |
| <pre class="prettyprint"> |
| |
| // 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 |
| </pre> |
| </li> |
| <li><span class="heading">RAII</span> |
| <p>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.</p> |
| |
| <p>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.</p> |
| </li> |
| |
| <li><span class="heading">Pointers and references</span> |
| <p>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.</p> |
| <p>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.</p> |
| </li> |
| |
| <li><span class="heading">Containers</span> |
| <p>TODO: describe stl container usage policies</p> |
| </li> |
| |
| <li><span class="heading">Exceptions</span> |
| <p>TODO: exceptions can be used, custom ones must be based on std::exception</p> |
| </li> |
| |
| <li><span class="heading">Polymorphism</span> |
| <p>TODO: when to use virtual functions, virtual destructor</p> |
| </li> |
| |
| <li><span class="heading">Namespaces</span> |
| <p>TODO: namespace naming</p> |
| <p>TODO: using statement, never using in headers</p> |
| </li> |
| |
| </ol> |
| </li> |
| |
| <li><span class="heading">Tools</span> |
| <ol class="h2"> |
| <li><span class="heading">Git</span> |
| <p>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.</p> |
| |
| <p>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.</p> |
| |
| <p>Before you commit anything, make sure <i>user.name</i> and <i>user.email</i> are properly set up.</p> |
| <pre class="prettyprint"> |
| git config --global user.name "Veijo Elements" |
| git config --global user.email "veijo.elements@drawelements.com" |
| </pre> |
| |
| <p>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 <i>core.autocrlf</i> to <i>input</i>. That causes |
| conversion to Unix-style line endings on commit only (i.e. not in checkout).</p> |
| <pre class="prettyprint"> |
| git config --global core.autocrlf input |
| </pre> |
| |
| <p>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.</p> |
| <pre class="prettyprint"> |
| # in repository directory |
| cp ~/Dropbox/drawElements/Misc/git/pre-commit .git/hooks/ |
| </pre> |
| </li> |
| |
| <li><span class="heading">Build systems and IDEs</span> |
| <p>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.</p> |
| |
| <p>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.</p> |
| </li> |
| </ol> |
| </li> |
| |
| <li><span class="heading">Coding philosophy</span> |
| <ol class="h2"> |
| <li><span class="heading">Designing classes</span> |
| <p>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 <a href="http://en.wikipedia.org/wiki/Single_responsibility_principle"> |
| single responsibility principle</a>. It is probably easier to grasp it with an example.</p> |
| |
| <p>Consider a <i>Texture2D</i> 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 <i>Texture2D</i> class, |
| or as a separate <i>Blitter</i> class. Same applies to things such as reading texture from a file, |
| clearing the texture to a certain color and so forth.</p> |
| |
| <div class="codeTitle">Texture class example.</div> |
| <pre class="prettyprint"> |
| 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); |
| </pre> |
| <p>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.</p> |
| |
| <p>When writing the internal implementation a lot of thought should be put on maintaining |
| consistent state, or more formally, <a href="http://en.wikipedia.org/wiki/Class_invariant">class invariant</a>. |
| 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.</p> |
| |
| <p>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 <i>Texture2D</i> 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.</i> |
| |
| <!-- TODO: code example --> |
| |
| </li> |
| |
| <li><span class="heading">Global state</span> |
| <p>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.</p> |
| |
| <p>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.</p> |
| |
| |
| </li> |
| |
| <li><span class="heading">Variables vs. immutable values</span> |
| <p>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 <i>register</i> 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.</p> |
| |
| <p>Functional languages declare variables immutable, i.e. they are not really <i>var</i>ying |
| 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.</p> |
| |
| <p>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 <i>const</i> qualifier.</p> |
| |
| <p>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.</p> |
| |
| <p>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.</i> |
| |
| <div class="codeTitle">Complex code example.</div> |
| <pre class="prettyprint"> |
| 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; |
| } |
| </pre> |
| </li> |
| |
| <li><span class="heading">Pure functions</span> |
| <p>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 <i>sin(x)</i> is pure |
| function as it always returns the same value for same argument value and does not cause any side effects.</p> |
| |
| <p>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.</p> |
| |
| <p>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 <i>memcpy()</i>.</p> |
| |
| <div class="codeTitle">Examples</div> |
| <pre class="prettyprint"> |
| // 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); |
| |
| </pre> |
| </li> |
| </ol> |
| |
| <!-- |
| Coding philosophy TODO: |
| - composition vs. inheritance |
| - dependency injection |
| - function design |
| - do not duplicate state (local or remote) |
| |
| Patterns TODO: |
| - iterator pattern |
| - iterate() pattern for long computation |
| + state machines for interactive processing? |
| - accessor class pattern |
| --> |
| |
| </li> |
| |
| <!--- |
| <li><span class="heading">Something else</span> |
| </li> |
| --> |
| |
| </ol> <!-- h1 --> |
| |
| </div> <!-- body --> |
| |
| </body> |
| |
| </html> |