Difference between revisions of "CodingStandards"
m (→Use Const: Formatting) |
m (→Write Exception-Safe Code: Formatting) |
||
Line 71: | Line 71: | ||
will result in c == 5 and i == 0 which may not be what you expect. | will result in c == 5 and i == 0 which may not be what you expect. | ||
− | === Write | + | === Write exception-safe code === |
− | Wesnoth code should be exception-safe, even if you do not use exceptions directly. | + | Wesnoth code should be exception-safe, even if you do not use exceptions directly. That is, you should be able to assume that an exception is thrown almost anywhere from within the code, with well-defined results (i.e. no resource leaks). |
− | That is, you should be able to assume that an exception is thrown almost anywhere | ||
− | from within the code, with well-defined results (i.e. no resource leaks). | ||
Code that uses a pattern like, | Code that uses a pattern like, | ||
− | + | { | |
− | + | SDL_Surface* image = IMG_Load("image.bmp"); | |
− | + | ...some code, which uses 'image'... | |
− | + | SDL_FreeSurface(image); | |
− | + | } | |
is bad, because the code may throw an exception, and 'image' will never be freed. | is bad, because the code may throw an exception, and 'image' will never be freed. | ||
Line 91: | Line 89: | ||
So you could rewrite the above code, | So you could rewrite the above code, | ||
− | + | { | |
− | + | surface image(IMG_Load("image.bmp")); | |
− | + | ...some code, which uses 'image'... | |
− | + | } ''the image is automatically freed here when 'image' is destroyed | |
− | Instead of allocating memory directly using new[] or malloc(), | + | Instead of allocating memory directly using new[] or malloc(), use language-provided containers, such as vector. |
− | use language-provided containers, such as vector. | ||
=== Do not use sprintf === | === Do not use sprintf === |
Revision as of 03:30, 25 February 2012
Wesnoth uses modern/advanced C++ that is portable to modern C++ compilers.
Contents
Formatting
When working on C++ for Wesnoth, indent your code with a tab character. After fully indenting, if you still need to line up the text with a specific character on the line above, you may further align it using space characters.
You may use long lines.
Evil Evil Things To Avoid
Avoid implicit conversions
Make all constructors which only take one argument that is of a different type to the class 'explicit'.
Do not use operator T() where T is a type to allow an implicit conversion to a different type.
Example:
t_string(const std::string&);
This is very evil! It can cause many situations where a temporary t_string is implicitly created and then gets destroyed unexpectedly.
Do not use non-private data members of classes
It's okay to have a struct with all-public members, if that's what you want.
However, once something is a class, with private data members, do not add public (or even protected) data members to the class. Doing this breaks encapsulation and can cause all kinds of confusing and evil things to happen.
Naming
End non-public class data members with an underscore
All non-public data members of classes should have their names terminated with an underscore, to show that they are a class member. This makes for more readable code, once one is familiar with the convention.
Idioms
Use references when a value may not be NULL
If a value passed to a function can never be NULL, use a reference instead of a pointer. I.e.
void myfunction(Object& obj);
rather than
void myfunction(Object* obj);
This more clearly shows the user of the function that obj may never be NULL, without them having to consult documentation or the implementation of the function.
Use the const keyword
The 'const' feature of C++ allows interfaces to more clearly specify how they treat objects. Always use const when you are not going to modify an object.
I.e.
void myfunction(const Object& obj);
demonstrates to the caller of myfunction() that obj will not be modified. If myfunction may modify obj, then use
void myfunction(Object& obj);
likewise, if a variable is not changed after initialization, make it const.
Know the behaviour of const references when types differ
If you assign something to a const reference of a different type, if necessary (if the type is different but there is a conversion) the compiler will create a temporary and guarantee it lasts for the lifetime of the reference. So
char c = 0; const int& i = c; c = 5;
will result in c == 5 and i == 0 which may not be what you expect.
Write exception-safe code
Wesnoth code should be exception-safe, even if you do not use exceptions directly. That is, you should be able to assume that an exception is thrown almost anywhere from within the code, with well-defined results (i.e. no resource leaks).
Code that uses a pattern like,
{ SDL_Surface* image = IMG_Load("image.bmp"); ...some code, which uses 'image'... SDL_FreeSurface(image); }
is bad, because the code may throw an exception, and 'image' will never be freed. Instead, use wrapper objects which free the object in their destructor.
For SDL_Surface objects, use the surface class. So you could rewrite the above code,
{ surface image(IMG_Load("image.bmp")); ...some code, which uses 'image'... } the image is automatically freed here when 'image' is destroyed
Instead of allocating memory directly using new[] or malloc(), use language-provided containers, such as vector.
Do not use sprintf
Sprintf does not check whether or not it is writing past the end of the space allocated. This is a security problem if someone other than the person running the game can cause sprintf to write very long strings. In Wesnoth this untrusted data could come potentially from other players in a multiplayer game or from downloaded campaigns. Instead you should use snprintf with the second argument being sizeof of the buffer that will hold the result.
Standard C++ to avoid
Do not use wstring
The standard C++ wstring class, defined as a basic_string< wchar_t >, does not exist in some platforms supported by Wesnoth. Use wide_string, defined in language.hpp, instead. wide_string is actually defined as a vector< wchar_t >
Do not use 0 when you mean NULL
Several Wesnoth developers, including Dave, find the number 0 to be very ambiguous when used in a non-numeric context. In keeping with the precedent that has already been established in the Wesnoth source code, you should avoid using literal zero for initializing and/or comparing null pointers.
C legacy to be avoided
Use util::array instead of C-style Arrays
C-style arrays are very efficient, but their interface is ugly. Use util::array defined in array.hpp. It is a wrapper for an array which has a C++ container-style interface. If you need to, extend it to make it fit your needs.
Do not use C-style casts
The following code,
if(i->second.side() == (size_t)player_number_) {
is considered bad practice in C++ since a C-style cast is overpowered -- if types change around it could end up casting away constness, or performing an implementation-defined data reinterpretation (basically a C-style cast is a compiler generated combination of static_cast, reinterpret_cast, and const_cast).
Good programming style is to use the least powerful tool available that does what you want. For example,
if(i->second.side() == static_cast<size_t>(player_number_)) {
Alternatively, a constructor call may be used for non-builtin types.
Note: there may be some obscure cases where a C-style cast is desirable, such as converting a pointer to an integer type of unspecified size.
Do not use #define for constant variable
#define foo X is not typesafe and you can use a const foo = X; (in an anonymous namespace) to achieve the same but typesafe.
Documentation
Document "config" preconditions and postconditions
In the Wesnoth code you will commonly encounter a data container known as the "config", which contains heirarchical string data (such as WML contents or game settings). The tagged "children" of the config and their string "attributes" are arranged in an ordered and mapped format internally using STL.
Because config data is utilized in so many ways and places, it can be difficult to track across the scope of the entire program. You should document all public functions that take/return a config, specifying config content expectations (and updating any related entries in the ReferenceWML wiki pages). In particular, if your function requires a config parameter, specify where/how the config should be created. This will be a great help to any future coders who need to call or modify your function.
Doxygen
See Doxygen for tips on how to comment the code, so that doxygen can nicely document it.