|
|
| (49 intermediate revisions by 4 users not shown) |
| Line 1: |
Line 1: |
| − | == C++ Standard ==
| + | {{Template:MovedToWebpage|https://vcmi.eu/developers/Coding_Guidelines/}} |
| − | VCMI implementation bases on C++03 standard with several extensions available in compilers we target (GCC 4.7.2+, MSVC 2013+, Clang 3.1+)
| |
| − | | |
| − | Further information about these extensions and compiler support for them is available at [http://wiki.apache.org/stdcxx/C%2B%2B0xCompilerSupport].
| |
| − | === Currently allowed C++11 features ===
| |
| − | * auto keyword,
| |
| − | * decltype,
| |
| − | * lambda expressions and closures,
| |
| − | * local and unnamed types as template arguments,
| |
| − | * new function declaration syntax for deduced return types (returned type after argument list),
| |
| − | * right angle brackets,
| |
| − | * r-value references and std::move,
| |
| − | * static assert,
| |
| − | * nullptr keyword,
| |
| − | * range-based for loops,
| |
| − | * forward enum declarations,
| |
| − | * strongly-typed enums,
| |
| − | * override and final,
| |
| − | * explicit conversion operators,
| |
| − | * initializer lists,
| |
| − | * raw string literals,
| |
| − | * variadic templates,
| |
| − | * defaulted and deleted functions (except for defaulted move functions),
| |
| − | * non-static data member initializers,
| |
| − | * extended friend Declarations,
| |
| − | * template aliases.
| |
| − | | |
| − | Note: due to bug in Visual Studio non-static data member initialization cannot be used together with the new brace-initialization syntax: http://blogs.msdn.com/b/vcblog/archive/2014/08/19/the-future-of-non-static-data-member-initialization.aspx
| |
| − | | |
| − | == Style Guidelines ==
| |
| − | | |
| − | | |
| − | In order to keep the code consistent, please use the following conventions. From here on `good' and `bad' are used to attribute things that would make the coding style match, or not match. It is not a judgment call on your coding abilities, but more of a style and look call. Please try to follow these guidelines to ensure prettiness.
| |
| − | | |
| − | | |
| − | === Indentation ===
| |
| − |
| |
| − | Use tabs for indentation. If you are modifying someone else's code, try to keep the coding style similar. Switch statements have the case at the same indentation as the switch.
| |
| − | | |
| − | === Switch statement ===
| |
| − | | |
| − | <syntaxhighlight lang="cpp">
| |
| − | switch(alignment)
| |
| − | { | |
| − | case EAlignment::EVIL:
| |
| − | do_that();
| |
| − | break;
| |
| − | case EAlignment::GOOD:
| |
| − | do_that();
| |
| − | break;
| |
| − | }
| |
| − | </syntaxhighlight>
| |
| − | | |
| − | === Where to put spaces ===
| |
| − | | |
| − | Use a space before and after the address or pointer character in a pointer declaration.
| |
| − | | |
| − | Good:
| |
| − | <syntaxhighlight lang="cpp">CIntObject * images[100];</syntaxhighlight>
| |
| − | | |
| − | Bad:
| |
| − | <syntaxhighlight lang="cpp">CIntObject* images[100]; or
| |
| − | CIntObject *images[100];</syntaxhighlight>
| |
| − | | |
| − | | |
| − | === Use whitespace for clarity ===
| |
| − | | |
| − | Use white space in expressions liberally, except in the presence of parenthesis.
| |
| − | | |
| − | '''Good:'''
| |
| − | <syntaxhighlight lang="cpp">if(a + 5 > method (blah ('a') + 4))
| |
| − | foo += 24;</syntaxhighlight>
| |
| − | | |
| − | '''Bad:'''
| |
| − | <syntaxhighlight lang="cpp">if(a+5>method(blah('a')+4))
| |
| − | foo+=24;</syntaxhighlight>
| |
| − | | |
| − | Between if, for, while,.. and the opening brace there shouldn't be a whitespace. The keywords are highlighted, so they don't need further separation.
| |
| − | | |
| − | === Where to put braces ===
| |
| − | | |
| − | Inside a code block put the opening brace on the next line after the current statement:
| |
| − | | |
| − | Good:
| |
| − | <syntaxhighlight lang="cpp">if(a)
| |
| − | { | |
| − | code ();
| |
| − | code ();
| |
| − | }</syntaxhighlight>
| |
| − | | |
| − | Bad:
| |
| − | <syntaxhighlight lang="cpp">if(a) {
| |
| − | code ();
| |
| − | code ();
| |
| − | }</syntaxhighlight>
| |
| − | | |
| − | Avoid using unnecessary open/close braces, vertical space is usually limited:
| |
| − | | |
| − | Good:
| |
| − | <syntaxhighlight lang="cpp">if(a)
| |
| − | code ();</syntaxhighlight>
| |
| − | | |
| − | Bad:
| |
| − | <syntaxhighlight lang="cpp">if(a) {
| |
| − | code ();
| |
| − | }</syntaxhighlight>
| |
| − | | |
| − | Unless there are either multiple hierarchical conditions being used or that the condition cannot fit into a single line.
| |
| − | | |
| − | Good:
| |
| − | <syntaxhighlight lang="cpp">if(a)
| |
| − | {
| |
| − | if(b)
| |
| − | code ();
| |
| − | }</syntaxhighlight>
| |
| − | | |
| − | Bad:
| |
| − | <syntaxhighlight lang="cpp">if(a)
| |
| − | if(b)
| |
| − | code ();</syntaxhighlight>
| |
| − | | |
| − | When defining a method, use a new line for the brace, like this:
| |
| − | | |
| − | Good:
| |
| − | <syntaxhighlight lang="cpp">void method()
| |
| − | {
| |
| − | }</syntaxhighlight>
| |
| − | | |
| − | Bad:
| |
| − | <syntaxhighlight lang="cpp">void Method() {
| |
| − | }</syntaxhighlight>
| |
| − | | |
| − | When allocating objects, don't use parentheses for creating stack-based objects by zero param c-tors to avoid c++ most vexing parse and use parentheses for creating heap-based objects.
| |
| − | | |
| − | Good:
| |
| − | <syntaxhighlight lang="cpp">std::vector<int> v;
| |
| − | CGBoat btn = new CGBoat();</syntaxhighlight>
| |
| − | | |
| − | Bad:
| |
| − | <syntaxhighlight lang="cpp">std::vector<int> v(); // shouldn't compile anyway
| |
| − | CGBoat btn = new CGBoat;</syntaxhighlight>
| |
| − | | |
| − | === File headers ===
| |
| − | | |
| − | For any new files, please paste the following info block at the very top of the source file:
| |
| − | <syntaxhighlight lang="cpp">/*
| |
| − | * Name_of_File.h, part of VCMI engine
| |
| − | *
| |
| − | * Authors: listed in file AUTHORS in main folder
| |
| − | *
| |
| − | * License: GNU General Public License v2.0 or later
| |
| − | * Full text of license available in license.txt file, in main folder
| |
| − | *
| |
| − | */</syntaxhighlight>
| |
| − | The above notice have to be included both in header and source files (.h/.cpp).
| |
| − | | |
| − | === Where and how to comment ===
| |
| − | | |
| − | You should write a comment before the class definition which describes shortly the class. 1-2 sentences are enough. Methods and class data members should be commented if they aren't self-describing only. Getters/Setters, simple methods where the purpose is clear or similar methods shouldn't be commented, because vertical space is usually limited. The style of documentation comments should be the three slashes-style: ///.
| |
| − | | |
| − | <syntaxhighlight lang="cpp">/// Returns true if a debug/trace log message will be logged, false if not.
| |
| − | /// Useful if performance is important and concatenating the log message is a expensive task.
| |
| − | bool isDebugEnabled() const;
| |
| − | bool isTraceEnabled() const;</syntaxhighlight>
| |
| − | | |
| − | The above example doesn't follow a strict scheme on how to comment a method. It describes two methods in one go. Comments should be kept short.
| |
| − | | |
| − | If you need a more detailed description for a method you can use such style:
| |
| − | <syntaxhighlight lang="cpp">/// <A short one line description>
| |
| − | ///
| |
| − | /// <Longer description>
| |
| − | /// <May span multiple lines or paragraphs as needed>
| |
| − | ///
| |
| − | /// @param Description of method's or function's input parameter
| |
| − | /// @param ...
| |
| − | /// @return Description of the return value</syntaxhighlight>
| |
| − | | |
| − | A good essay about writing comments: [http://ardalis.com/when-to-comment-your-code]
| |
| − | | |
| − | === Casing ===
| |
| − |
| |
| − | | |
| − | Local variables and methods start with a lowercase letter and use the camel casing. Classes/Structs start with an uppercase letter and use the camel casing as well. Macros and constants are written uppercase.
| |
| − | | |
| − | | |
| − | === Line length ===
| |
| − | | |
| − | The line length for c++ source code is 120 columns. If your function declaration arguments go beyond this point, please align your arguments to match the opening brace. For best results use the same number of tabs used on the first line followed by enough spaces to align the arguments.
| |
| − | | |
| − | | |
| − | === Warnings ===
| |
| − | | |
| − | Avoid use of #pragma to disable warnings. Compile at warning level 3.
| |
| − | Avoid commiting code with new warnings.
| |
| − | | |
| − | === Type naming ===
| |
| − | | |
| − | Classes are prefixed with an upper C, interfaces with an upper I, enumerations with an upper E, structs without an prefix and typedefs with an upper T
| |
| − | | |
| − | === File/directory naming ===
| |
| − | | |
| − | Compilation units(.cpp,.h files) start with a uppercase letter and are named like the name of a class which resides in that file if possible. Header only files start with a uppercase letter. JSON files start with a lowercase letter and use the camel casing.
| |
| − | | |
| − | Directories start with a lowercase letter and use the camel casing where necessary.
| |
| − | | |
| − | | |
| − | === Logging ===
| |
| − | | |
| − | If you want to trace the control flow of VCMI, then you should use the macro LOG_TRACE or LOG_TRACE_PARAMS. The first one prints a message when the function is entered or leaved. The name of the function will also be logged. In addition to this the second macro, let's you specify parameters which you want to print. You should print traces with parameters like this:
| |
| − | | |
| − | <syntaxhighlight lang="cpp">LOG_TRACE_PARAMS(logGlobal, "hero '%s', spellId '%d', pos '%s'.", hero, spellId, pos);</syntaxhighlight>
| |
| − | | |
| − | When using the macro every "simple" parameter should be logged. The parameter can be a number, a string or a type with a ostream operator<<. You should not log contents of a whole text file, a byte array or sth. like this. If there is a simple type with a few members you want to log, you should write an ostream operator<<. The produced message can look like this:
| |
| − | | |
| − | <tt>{BattleAction: side '0', stackNumber '1', actionType 'Walk and attack', destinationTile '{BattleHex: x '7', y '1', hex '24'}', additionalInfo '7', selectedStack '-1'}</tt>
| |
| − | | |
| − | The name of the type should be logged first, e.g. {TYPE_NAME: members...}. The members of the object will be logged like logging trace parameters. Collection types (vector, list, ...) should be logged this way: [{BattleHex: ...}, {...}]
| |
| − | There is no format which has to be followed strictly, so if there is a reason to format members/objects in a different way, then this is ok.
| |
| − | | |
| − | == Best practices ==
| |
| − | | |
| − | | |
| − | === Avoid code duplication ===
| |
| − | | |
| − | Avoid code duplication or don't repeat yourself(DRY) is the most important aspect in programming. Code duplication of any kind can lead to inconsistency and is much harder to maintain. If one part of the system gets changed you have to change the code in several places. This process is error-prone and leads often to problems. Here you can read more about the DRY principle: [http://en.wikipedia.org/wiki/Don%27t_repeat_yourself http://en.wikipedia.org/wiki/Don%27t_repeat_yourself]
| |
| − | | |
| − | === Do not use uncommon abbrevations ===
| |
| − | | |
| − | Do not use uncommon abbrevations for class, method, parameter and global object names.
| |
| − | | |
| − | Bad:
| |
| − | <syntaxhighlight lang="cpp">
| |
| − | CArt * getRandomArt(...)
| |
| − | class CIntObject
| |
| − | </syntaxhighlight>
| |
| − | | |
| − | Good:
| |
| − | <syntaxhighlight lang="cpp">
| |
| − | CArtifact * getRandomArtifact(...)
| |
| − | class CInterfaceObject
| |
| − | </syntaxhighlight>
| |
| − | | |
| − | === Loop handling ===
| |
| − | | |
| − | Use BOOST_FOREACH for iterating through every item of a container. It should be used in any case except if Visual Studio debug performance is important, then you may use a simple for loop to avoid use of STL Iterator checks.
| |
| − | | |
| − | The loop counter should be of type int, unless you are sure you won't need negative indices -- then use size_t.
| |
| − | | |
| − | === Include guards ===
| |
| − | | |
| − | Use #pragma once instead of the traditional #ifndef/#define/#endif include guards.
| |
| − | | |
| − | | |
| − | === Pre compiled header file ===
| |
| − | | |
| − | The header StdInc.h should be included in every compilation unit. It has to be included before any C macro and before any c++ statements. Pre compiled header should not be changed, except any important thing is missing.
| |
| − | The StdInc includes most Boost libraries and nearly all standard STL and C libraries, so you don’t have to include them by yourself.
| |
| − | | |
| − | | |
| − | === Enumeration handling ===
| |
| − | | |
| − | Do not declare enumerations in global namespace. It is better to use strongly typed enum or to wrap them in class or namespace to avoid polluting global namespace:
| |
| − | | |
| − | <syntaxhighlight lang="cpp">
| |
| − | | |
| − | enum class EAlignment { GOOD, EVIL, NEUTRAL };
| |
| − | | |
| − | namespace EAlignment
| |
| − | {
| |
| − | enum EAlignment { GOOD, EVIL, NEUTRAL };
| |
| − | } | |
| − | </syntaxhighlight>
| |
| − | | |
| − | === Avoid senseless comments ===
| |
| − | | |
| − | If the comment duplicates the name of commented member, it's better if it wouldn't exist at all. It just increases maintenance cost.
| |
| − | Bad:
| |
| − | <syntaxhighlight lang="cpp">size_t getHeroesCount(); //gets count of heroes (surprise?)</syntaxhighlight>
| |
| − | | |
| − | | |
| − | === Class handling ===
| |
| − | | |
| − | There is no definitive rule which has to be followed strictly. You can freely decide if you want to pack your own classes, where you are programming on, all in one file or each in one file. It's more important that you feel comfortable with the code, than consistency overall the project. VCMI has several container class files, so if you got one additional class to them than just add it to them instead of adding new files.
| |
| − | | |
| − | === Functions and interfaces ===
| |
| − | | |
| − | Don't return const objects or primitive types from functions -- it's pointless. Also, don't return pointers to non-const game data objects from callbacks to player interfaces.
| |
| − | | |
| − | Bad:
| |
| − | <syntaxhighlight lang="cpp">
| |
| − | const std::vector<CGObjectInstance*> guardingCreatures (int3 pos) const;
| |
| − | </syntaxhighlight>
| |
| − | | |
| − | Good:
| |
| − | <syntaxhighlight lang="cpp">
| |
| − | std::vector<const CGObjectInstance*> guardingCreatures (int3 pos) const;
| |
| − | </syntaxhighlight>
| |
| − | | |
| − | == Bug tracker usage ==
| |
| − | VCMI maintains bug tracker at [http://bugs.vcmi.eu]. All developers should be registered here and get proper access rights (can be granted by Tow or Warmonger).
| |
| − | === Issue workflow ===
| |
| − | Typical workflow can be described with this table:
| |
| − | {|
| |
| − | ! width=200 | -> New || Once issue is reported it will be in "new" status. Note that for some categories issue will be automatically assigned to a specific developer
| |
| − | |-
| |
| − | ! width=200 | New -> Assigned || Developer who is interested in resolving this issue (or started working on it) should assign issue to himself
| |
| − | |-
| |
| − | ! width=200 | Assigned -> Feedback || If information supplied by reported is incomplete its status should be changed to feedback for clarification
| |
| − | |-
| |
| − | ! width=200 | Feedback -> Assigned || Once original reporter responds to an issue Mantis will automatically change its status back to resolved. Othervice (if clarification came from somebody else) this should be done manually
| |
| − | |-
| |
| − | ! width=200 | Assigned -> Resolved || Once issue is resolved and fix has entered main repository (vcmi/develop on github) it should be marked as resolved by developer, "fixed in version" field changed to next release codename "0.next" and confirmation message should contain link to git commit fixing this issue.
| |
| − | |-
| |
| − | ! width=200 | Resolved -> Closed || After reporter confirms that issue is resolved its status should be changed to closed.
| |
| − | |-
| |
| − | ! width=200 | <any> -> Closed || If issue is won't be resolved by VCMI Team (rejected features, duplicates, user faults, etc) it should be marked as closed.
| |
| − | |}
| |
| − | | |
| − | Note on confirmed/acknowledged status: their usage is not clearly defined and usually used to:
| |
| − | * Acknowledged is used to indicate valid issues that won't be fixed soon ("I can confirm this but can't fix it in near future because ...")
| |
| − | * Confirmed is used to indicate that developer knows origin of a bug (usually used for issues with random reproducibility)
| |
| − | == Sources ==
| |
| − | [http://www.mono-project.com/Coding_Guidelines Mono project coding guidelines]
| |
