Changes between Version 6 and Version 7 of Coding_Conventions


Ignore:
Timestamp:
Nov 24, 2011, 1:38:06 PM (12 years ago)
Author:
Philip Taylor
Comment:

start cleaning up coding conventions

Legend:

Unmodified
Added
Removed
Modified

  • Coding_Conventions

    v6 v7  
    1 (Note: This document is aimed at C++ code. We should write something similar for JS.)
     1[[TOC(inline)]]
    22
    33== Introduction ==
    44
    55The goal of these coding conventions is to encourage consistency within the code, rather than claiming some particular style is better than any other. As such, the main guideline is:
    6  * When modifying a piece of code, try to follow its existing style.
    7 Our code is currently self-inconsistent in places, but the following guidelines attempt to describe the most common style and are what we should converge towards. Obviously we want clean, readable, adequately-documented code - lots of articles and books already talk about how to do that - so we mostly describe boring typographic details.
     6 * When modifying a piece of code, try to follow its existing style. In particular:
     7  * Primarily, try to match the style of the functions that you're editing (assuming it's at least self-consistent and not too bizarre), in order to avoid making it less self-consistent.
     8  * Secondly, try to match the style of the files that you're editing.
     9  * Then, try to match the style of the other code in the subdirectory you're editing.
     10  * Finally, try to match the global guidelines discussed on this page.
    811
    9 == Brief guidelines ==
     12Our code is currently not entirely consistent in places, but the following guidelines attempt to describe the most common style and are what we should converge towards. (Obviously we always want clean, readable, adequately-documented code - lots of articles and books already talk about how to do that - so here we're mostly describing minor details.)
    1013
    11  * All source files (.cpp, .h) must start with the following header, before any other content:
     14== C++ ==
     15
     16=== Creating new files ===
     17
     18 * All source files (.cpp, .h) must start with the following GPL license header, before any other content:
    1219{{{
    13 /* Copyright (C) 2010 Wildfire Games.
     20/* Copyright (C) 2011 Wildfire Games.
    1421 * This file is part of 0 A.D.
    1522 *
    … …  
    2835 */
    2936}}}
    30 replacing `2010` with the year that the file was last updated.
     37 replacing `2011` with the year that the file was last updated.
     38
     39 ''Exception:'' Code in `source/lib/` (and a few other files) should use the MIT license instead:
     40{{{
     41/* Copyright (c) 2011 Wildfire Games
     42 *
     43 * Permission is hereby granted, free of charge, to any person obtaining
     44 * a copy of this software and associated documentation files (the
     45 * "Software"), to deal in the Software without restriction, including
     46 * without limitation the rights to use, copy, modify, merge, publish,
     47 * distribute, sublicense, and/or sell copies of the Software, and to
     48 * permit persons to whom the Software is furnished to do so, subject to
     49 * the following conditions:
     50 *
     51 * The above copyright notice and this permission notice shall be included
     52 * in all copies or substantial portions of the Software.
     53 *
     54 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
     55 * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
     56 * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
     57 * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
     58 * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
     59 * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
     60 * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
     61 */
     62}}}
    3163
    3264 * Wrap header files in include guards, using the name `INCLUDED_`''filename'', e.g. the file `Foo.h` should say:
    … …  
    4274 * The first non-comment line of any source file must be `#include "precompiled.h"`
    4375
    44  * In header files, avoid `#include` and use forward declarations wherever possible.
     76=== Formatting ===
    4577
    4678 * Use tabs for indentation, not spaces.
    4779
    48  * Indent braces like
     80 * For any alignment within a line of code (as opposed to indentation at the start), use spaces, not tabs.
     81
     82 * Indent braces and use whitespace like
    4983{{{
    5084int CExampleObject::DoSomething(int value)
    … …  
    5892}
    5993}}}
     94 ''Exception:'' Code in `source/lib/` omits the space before the '`(`' in statements like "`if(...)`", "`while(...)`", etc.
    6095
    61  * Class names are !CamelCase and prefixed with `C`, e.g. `CGameObject`. Member functions are !CamelCase, e.g. `CGameObject::SetModifiedFlag(...)`. Member variables are !CamelCase prefixed with `m_`, e.g. `CGameObject::m_ModifiedFlag`. Files are named `GameObject.cpp`, `GameObject.h`, usually with one major class per file (possibly with some other support classes in the same files).
     96 * Try to avoid very wide lines. Typical wrapping points are 80 characters, or 120, or 132, etc. (There's no strict limit - aim for whatever seems most readable.)
    6297
    63  * Use [http://www.doxygen.org/ Doxygen] comments, e.g.
     98=== Documentation ===
     99
     100 * Use [http://www.doxygen.org/ Doxygen] comments (explained [http://www.stack.nl/~dimitri/doxygen/docblocks.html here] as !JavaDoc style), e.g.
    64101{{{
     102/**
     103 * A dull object for demonstrating comment syntax.
     104 */
    65105class CExampleObject
    66106{
    … …  
    73113     */
    74114    int DoSomething(int v);
     115
     116    /// Current value (always non-zero)
     117    int m_Value;
    75118};
    76119}}}
    77120
    78  * Use STL when appropriate.
     121 * Try not to repeat class names or function names in the descriptions, since that's redundant information.
    79122
    80  * ...
     123 * Don't need to bother documenting every line of code or every member function or every member variable; only when it'll add to a competent reader's understanding of the program.
    81124
    82 == Strings ==
     125=== Strings ===
    83126
    84 Use `CStr` instead of `std::string`. Use `CStrW` instead of `std::wstring`.
     127 * Use `CStr` instead of `std::string`. Use `CStrW` instead of `std::wstring`. (These are subclasses of `std::[w]string` with various extra methods added for conveniene.)
    85128
    86 For portability, use the following formats for printf-style functions:
     129 * For portability, use the following formats for printf-style functions:
    87130{{{
    88131printf("%s", "char string");
    … …  
    92135}}}
    93136
    94 = Old document =
     137=== Misc ===
    95138
    96 ''The following content might be somewhat incorrect or outdated, but it's preserved here since it contains some useful information.''
     139 * In header files, avoid `#include` and use forward declarations wherever possible.
    97140
    98 == Objective ==
     141 * Class names are !CamelCase and prefixed with `C`, e.g. `CGameObject`. Member functions are !CamelCase, e.g. `CGameObject::SetModifiedFlag(...)`. Member variables are !CamelCase prefixed with `m_`, e.g. `CGameObject::m_ModifiedFlag`. Files are named `GameObject.cpp`, `GameObject.h`, usually with one major class per file (possibly with some other support classes in the same files).
    99142
    100 The goal of this document is to provide a standardized coding methodology for the 0 A.D. programming team. With but a few guidelines for layout, commenting and naming conventions, the team should feel as if they are reading their own code when reading someone else's code.
     143 * Use STL when appropriate.
    101144
    102 == Layout ==
     145 * Don't use RTTI (`dynamic_cast` etc). ''Exception:'' `source/tools/atlas/AtlasUI/` can use RTTI.
    103146
    104 === Formatting ===
    105 
    106 Most editors allow for the conversion of tabs to spaces and most people prefer the use of tabs versus wearing out the spacebar. The size of the tabs is up to each programmer -- just ensure that you are using tabs.
    107 
    108 Limit the length of a line of code to not more than 80 characters; not everyone has a 1600x1200 display. Functions that have many parameters and extend over 80 characters should be written as:
    109 {{{
    110    SomeFunction(
    111        HWND hWnd,
    112        BITMAP bmDeviceBitmap,
    113        long lAnimationFrame);
    114 }}}
    115 instead of:
    116 {{{
    117    SomeFunction(HWND hWnd,
    118                 BITMAP bmDeviceBitmap,
    119                 long lAnimationFrame);
    120 }}}
    121 Although the second method is commonly used, it is more difficult to maintain (if the name of the function changed, you would need to re-align the parameters).
    122 
    123 === Brackets ===
    124 
    125 Brackets should be aligned, here's an example of good bracket placement:
    126 {{{
    127    void CGameObject::CleanUp()
    128    {
    129        if(NULL != m_ThisObject)
    130        {
    131            delete m_ThisObject;
    132        }
    133    }
    134 }}}
    135 Now we're not out to save vertical lines on the screen; it's about being able to read the code. Therefore, the following style should be avoided:
    136 {{{
    137    void CGameObject::CleanUp() {
    138        if(NULL != m_ThisObject) {
    139            delete m_ThisObject;
    140        }
    141    }
    142 }}}
    143 
    144 == Commenting ==
    145 
    146 Commenting is a subject that is sure to cause debate, but minimal comments with maximum expressiveness are preferable. Bad commenting style is shown below:
    147 {{{
    148    void CGameObject::SetModifiedFlag(bool flag)
    149    {
    150        m_ModifiedFlag = flag;    // set the modified flag
    151    }
    152 }}}
    153 The above comment does not tell us anything that we don't already know from reading the code; here's a better approach:
    154 {{{
    155    void CGameObject::SetModifiedFlag(bool flag)
    156    {
    157        // This sets the CGameObject's modified
    158        // flag, which is used to determine
    159        // if this object needs to be serialized.
    160        m_ModifiedFlag = flag;
    161    }
    162 }}}
    163 
    164 == Naming Conventions ==
    165 
    166 === Filenames ===
    167 
    168 Filenames can be freely chosen, but to avoid problems on Unix systems, they should not contain spaces or non-ASCII characters. If the file serves to define one class, e.g. `CEntity`, the file would usually be called `Entity.h`.
    169 
    170 === Namespaces ===
    171 
    172 Namespaces are used as a mechanism to express logical grouping.
    173 
    174 ==== Global Scope ====
    175 
    176 Symbols belonging to the global namespace should be prefixed with `::`.
    177 Example: The Win32 function `::OutputDebugString()` resides in the global namespace and is written with the scope operator preceding the function name.
    178 
    179 === Classes ===
    180 
    181 Classes should use concise, descriptive names that easily convey their use.
    182 
    183 Classes are named using !PascalCase - capitalizing each word within the name visually differentiates them. Example: A class named `CGameObject` is preferred over `gameObject` or `cGameObject`.
    184 
    185 === Functions ===
    186 
    187 Functions should use concise, descriptive names that provide innate clues as to the functionality they provide.
    188 
    189 Global and member functions should be named using !PascalCase. Example: A function named `SetModifiedFlag()` is preferred over `SetFlag()` or `setFlag`.
    190 
    191 === Variables ===
    192 
    193 Variable should use concise, descriptive names that provide innate clues as to the data that the variable represents.
    194 
    195 Member variables should be prefixed with `m_`, but both `m_camelCase` and `m_PascalCase` may be used according to personal preference (either way, the prefix ensures clarity). Example: `m_GameObject` is more descriptive than `gobj`.
    196 
    197 == Documentation ==
    198 
    199 Each programmer is responsible for properly documenting their code. During code review the code reviewer will ensure that interfaces or APIs are properly documented.
    200 
    201 If the comments are formatted in a certain way, they will automatically be extracted and added to the relevant documentation file. It suffices to write them as follows (sample comment for a class):
    202 {{{
    203    /**
    204     * An object that represents a civilian entity.
    205     *
    206     * (Notes regarding usage and possible problems etc...)
    207     */
    208 }}}
    209 For single-line comments, `///` can be used as well. The comment text is inserted into the documentation, and can additionally be formatted by certain tags (e.g. `@param description` for function parameters). For more details, see the !Doxygen documentation.
    210 
    211 Each method of a class should be documented as well and here is the suggested method of documenting a member function (continuing with `CExample`):
    212 {{{
    213    class CExample
    214    {
    215    public:
    216        CExample();
    217        ~CExample():
    218 
    219        /**
    220         * This function does nothing, but is a good example of
    221         * documenting a member function.
    222         * @param dummy A dummy parameter.
    223         */
    224        void ShowExample(int dummy);
    225    
    226    private:
    227        intptr_t m_ExampleData;          // Holds the value of this example.
    228        double m_FairlyLongVariableName; // Shows the lining up of comments
    229    };
    230    }}}
    231 The ctor and dtor need not be commented --- everyone knows what they are and what they do. `ShowExample()`, on the other hand, provides a brief comment as to its purpose. You may also want to provide an example of a method's usage. Member data is commented on the right side and it is generally good (when possible) to line up comments for easier reading.
    232 
    233 === Author and Modified By ===
    234 
    235 To promote collective code ownership and encourage making necessary fixes to modules that happen to be written by others, we will avoid explicitly mentioning the author at the top of the file. Such a tag could (subconsciously) be interpreted as "his code only", a condition we want to avoid because "he" might get run over by a bus (thus losing the only source of knowledge and expertise on that piece of code).
    236 
    237 On a similar note, we will also avoid modified-by tags. The reasoning is as above, with the additional wrinkle of having to figure out when exactly to consider oneself to have modified the code. Is it after a quick typo fix, adding 2 lines, a function, ...?
    238 
    239 Note that the authors of course retain copyright; we can also reconstruct the file history and find out all contributors via SVN revision information. The purpose of this measure is simplicity and improved cooperation and does not deprive anyone of credit.
    240 
    241 
    242 === Example ===
    243 
    244 Here is a sample header file layout, `Example.h`:
    245 {{{
    246    /**
    247     * =========================================================================
    248     * File        : Example.h
    249     * Project     : 0 A.D.
    250     * Description : CExample interface file.
    251     * =========================================================================
    252     */
    253    
    254    /*
    255    This interface is difficult to write as it really
    256    pertains to nothing and serves no purpose other than to
    257    suggest a documentation scheme.
    258    */
    259 
    260    #ifndef INCLUDED_EXAMPLE
    261    #define INCLUDED_EXAMPLE
    262    
    263    #include "utils.h"
    264    
    265    /**
    266     * CExample
    267     * This serves no purpose other than to
    268     * provide an example of documenting a class.
    269     * Notes regarding usage and possible problems etc...
    270     */
    271    class CExample
    272    {
    273    public:
    274        CExample();
    275        ~CExample():
    276    
    277        /**
    278         * This function does nothing, but is a good example of
    279         * documenting a member function.
    280         * @param dummy A dummy parameter.
    281         */
    282        void ShowExample(int dummy);
    283    
    284    protected:
    285        int m_UsefulForDerivedClasses;
    286    
    287    private:
    288        uint8_t m_ExampleData;        // Holds the value of this example.
    289        int m_RatherLongVariableName; // Shows the lining up of comments
    290    };
    291    
    292    #endif    // #ifndef INCLUDED_EXAMPLE
    293 }}}
    294 From the above we can see that header guards are utilized. Header file comment blocks show filename, project and author; a short overview follows.
    295 
    296 The order of declarations ought to be: public followed by protected and finally by private.
    297  
    298 == Standard Template Library ==
    299 
    300 We will make use of the Standard Template Library (STL). Although we may be capable of coding list, maps and queues ourselves and do so more efficiently. Our goal is to create a game not to recreate an existing library.
    301 
    302 Having said that, it may make sense to hide some uses of STL objects behind an interface. This can make the code more readable.
    303 
    304 == Singletons ==
    305 
    306 After a long debate, it was decided to not use Singletons and to prefer global variables to them. All global variables have to be prefixed with 'g_' and their initialisation should be as clear and transparent as possible. So, if there's a class like CGame, which needs to exist in a single instance, there will be a pointer declaration called 'g_Game' in Game.h, and all initialisation details will be "hidden" in Game.cpp. There are still some Singletons in the current codebase, but they will be reimplemented and the new code should be written using globals.
    307 
    308 == Strings ==
    309 
    310 A string class has been written, `CStr`, that should be used instead of directly using `std::string` or using C-style strings (i.e. `char*`).
     147 * Avoid global state: global variables, static variables inside functions or inside classes, and singletons.
     148  * When a module needs access to objects from outside its own environment, prefer to pass them in explicitly as arguments when instantiating that module, rather than making the objects global and having the module reach out to grab them.
     149  * When unavoidable, global variables should be named with a `g_` prefix.
     150  * Prefer global variables over singletons, because then they're not trying to hide their ugliness.