The Battle for Wesnoth Wiki - User contributions [en]

HackingWesnoth

2012-10-20T11:03:26Z

Jesyspa: /* C++ Quiz */

This page is designed to be a guide for aspiring Wesnoth programmers on what they need to do to be able to productively contribute to the Wesnoth code base.

Wesnoth is written in C++, a large and complicated language. There are many C++ programmers, but they tend to have widely varying levels of skills and styles. This page is designed to be a guide as to the skillset needed to contribute to Wesnoth.

== C++ Quiz ==

Below is a short C++ quiz that assesses your skills in areas of C++ that are used a lot in Wesnoth. If you know all the answers to the questions, you are probably ready to start working on the Wesnoth code base. If you know most of the answers to the questions, you can probably work on the Wesnoth code base with some oversight from the senior developers. There are good C++ guides for both knowledgeable programmers (such as [http://www.icce.rug.nl/documents/cplusplus/ The C++ Annotations]) and beginners (such as one of the books [http://stackoverflow.com/q/388242/559931 suggested here]).

# What is a virtual destructor? Why is it needed?
# What does the standard class template <tt>auto_ptr</tt> do? What is its purpose?
# What is a vector, and why would you use it?
# What is a map, and why would you use it?
# What are the differences between a reference and a pointer? When should each be used?
# What is an iterator, and when is it used?
# What are the memory areas in a C++ program, and what is the purpose of each?
# When should a member function be <tt>const</tt>? What effect does this have?
# What is a copy constructor, and what is an assignment operator? When must you define them in a class?

== Wesnoth Coding Style ==

=== Guideline 1: Don't prematurely optimize ===

<blockquote>When I was a new and inexperienced coder, I read the following rather famous saying: "Premature optimization is the root of all evil." -- Donald Knuth.</blockquote>

While I wouldn't then -- and would be hesitant to now -- dispute the opinion of Donald Knuth on any programming topic, this absolute statement confused me. Premature optimization certainly doesn't sound like a wise thing to do, but it sounds somewhat obscure to be the root of all problems in programming. Had Knuth written this phrase after spending too long correcting problems caused by a programmer who was hell-bent on optimization at every turn?

Now I realize that Knuth was correct. Time and time again programming problems spring from developers making their code too complicated because they are trying to optimize unnecessarily.

By the [http://en.wikipedia.org/wiki/Pareto_principle Pareto Principle] we know that a minority of the code will occupy a majority of the running time. That is, perhaps 90% of the running time of the program will be spend executing just 5% of the code. It is thus generally unnecessary to apply many optimizations at all to most of the program. One can just write the code in a way that is the most readable and maintainable and forget about optimization altogether most of the time.

Remember also, C++ compilers are good, and smart, and they can execute code fast. Most code will run fast enough, unless you go out of your way to make it run slowly.

So, the first rule of Wesnoth programming is that unless you have a very good reason to think you need to optimize, do whatever is simplest and most maintainable.

Many programmers seem to have an obsession with using the control C++ gives them to do all sorts of crazy things. Like make their own memory management routines, or write their own containers, and so forth. Simply, don't. It's possible to write good C++ code very fast using the standard components supplied as part of the language.

This leads us to our next topic...

=== Guideline 2: Know the Standard containers and use them ===

One of the best things about C++ is that the standard supplies some great containers. Always prefer to use them first to store data. That is,

* prefer to use std::vector to store dynamic arrays
* prefer to use std::string to store strings (we also provide t_string for a translatable string -- it's based on std::basic_string, which std::string is also based on)
* prefer to use std::map to store key/value pairs
* prefer to use std::set to store sets of items (roughly equivalent to the mathematical concept of a set).

There are other C++ containers, and you should know about all of them. A good source of documentation on the portion of the C++ standard library known as the Standard Template Library (STL) which contains the standard containers can be found [http://www.sgi.com/tech/stl/ here].

By using the standard containers, you should almost never have to manually allocate and delete memory yourself by hand. The containers will automatically allocate and deallocate memory for you. This leads us to our next guideline...

=== Guideline 3: Use the resource acquisition is initialization (RAII) idiom ===

In C, you might write some code like this:

<pre>
void myfunction()
{
char* mystring = (char*)malloc(n);
/*some code, which manipulates mystring*/
...
free(mystring);
}
</pre>

This code works, but is error prone. What if the code in the middle exited using a return statement, or a longjmp statement, or in C++, threw an exception? The memory allocated would never be cleaned up, and we would have a memory leak.

Sure, you're a good programmer, and you probably make sure you're very careful about freeing the string. But what about when this function grows to be 100 lines long (and functions have a way of doing that), and then another programmer who isn't that familiar with the function is trying to fix a critical bug as quickly as they can, that requires adding an early return statement to this function? What about when someone is trying to read the code to scan for memory leaks? Once they see the malloc at the top, they will have to carefully read the entire function, to make sure it's all okay.

Now if we had this C++ version instead,

<pre>
void myfunction()
{
std::string mystring(n,'x');
/*some code, which manipulates mystring*/
} //mystring is destroyed and memory automatically released
</pre>

None of these problems can possibly happen. mystring will ''always'' be released at the end of the function.

So, this just re-iterates how important C++ containers are, right? Well, yes, but it also shows us that we should always avoid code written like this:

<pre>
void myfunction()
{
...allocate resources...
...use resources
...release resources...
}
</pre>

If you have code like this, you should put the resources in a class that will manage them and release them at the end of the function. If there is no class like that, then you should write one. This goes for all resources: memory, files, threads, and so forth.

It might sound like a silly mistake that we're protecting against here, one that an experienced programmer wouldn't make. However, almost all programming bugs are due to 'silly mistakes'. This leads us to our next item...

=== Guideline 4: Lean on the compiler as heavily as you can ===

Humans make mistakes. Even very stupid mistakes. Computers are as dumb as toast, but they are at least very very consistent.

For Wesnoth, we try to write code that will make the compiler guard against mistakes as much as possible. The use of RAII as described above is one example of this: writing the code so the compiler and language rules will guarantee it's correct.

There are some more ways to do this. One big way is to make all code const correct. That is, always define something as 'const' when it shouldn't change its value. If you have a pointer that is meant to be read only, define it as a const pointer. Then if the code is later changed so that your pointer changes the value, the compiler will produce an error.

Another way is to avoid implicit conversions between objects, and always use the C++-style casts.

== See also ==
* [[CodingStandards]]
* [[PatchSubmissionGuidelines]]

[[Category:Development]]

CodingStandards

2012-10-20T10:48:30Z

Jesyspa: /* Use the const keyword */

Wesnoth uses modern/advanced C++ that is portable to modern C++ compilers targeting various commonly used platforms.

== 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 things to avoid ==

=== Avoid implicit conversions ===

Make all constructors which only take one argument that is of a different type to the class <tt>explicit</tt>.

Do not use <tt>operator T()</tt> (where <tt>T</tt> is a type) to allow an implicit conversion to a different type. For example:

t_string(const std::string&);

This can cause many situations where a temporary t_string is implicitly created and then gets destroyed unexpectedly.

=== Do not declare class data members as non-private ===

It's okay to have a ''struct'' with only 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 <tt>NULL</tt>, use a reference instead of a pointer. For example:

void my_function(T& obj);

rather than

void my_function(T* obj);

This more clearly shows prospective users of the function that <tt>obj</tt> may never be <tt>NULL</tt>, without them having to consult documentation or the implementation of the function.

=== Use the const keyword ===

The <tt>const</tt> keyword in C++ allows interfaces to more clearly specify how they treat objects.
Always use <tt>const</tt> when you are not going to modify an object. For example:

void my_function(const T& obj);

This shows to the caller that <tt>obj</tt> will not be modified. If <tt>my_function()</tt> may modify <tt>obj</tt>, then use the following instead:

void my_function(T& obj);

Likewise, if a variable is not changed after initialization, make it <tt>const</tt>, and mark member functions as <tt>const</tt> if they do not modify their object.

=== Know the behavior of const references when types differ ===

If you assign something to a <tt>const</tt> 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 the following is bad:

{
SDL_Surface* image = IMG_Load("image.bmp");
...some code, which uses 'image'...
SDL_FreeSurface(image);
}

The code may throw an exception, and <tt>image</tt> will never be freed. Instead, use wrapper objects which free the object in their destructor.

For <tt>SDL_Surface</tt> objects, the <tt>surface</tt> type is used throughout the Wesnoth source code to achieve this purpose. So you could rewrite the above code as follows:

{
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 <tt>new[]</tt> or <tt>malloc()</tt>, use language-provided containers, such as vector.

=== Do not use sprintf ===

The <tt>sprintf()</tt> function 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 <tt>sprintf()</tt> to write very long strings. In Wesnoth, this untrusted data could come potentially from other players in a multiplayer game, or from downloaded add-ons. Instead you should use <tt>snprintf()</tt> with the second argument being the <tt>sizeof</tt> of the buffer that will hold the result.

== Standard C++ to avoid ==

=== Do not use wstring ===

The standard C++ <tt>std::wstring</tt> class (defined as a <tt>std::basic_string< wchar_t ></tt>) does not exist in some platforms supported by Wesnoth. Use <tt>wide_string</tt> instead (defined in <tt>language.hpp</tt>). The <tt>wide_string</tt> type is actually defined as <tt>std::vector< wchar_t ></tt>.

=== 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 <tt>util::array</tt> defined in <tt>array.hpp</tt> instead. 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 <tt>static_cast</tt>, <tt>reinterpret_cast</tt>, and <tt>const_cast</tt>).

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-built-in 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 constants ===

<tt><nowiki>#</nowiki>define foo X</tt> is not a typesafe approach to define constants. Instead, you can something like the following (in an anonymous namespace) to achieve the same goal in a typesafe fashion.

namespace {
const T foo = X;
}

== Documentation ==

=== Document config preconditions and postconditions ===

In the Wesnoth code you will commonly encounter a data container type known as <tt>config</tt>, which contains hierarchical string data (such as WML contents or game settings). The tagged ''children'' of the <tt>config</tt> object and their string ''attributes'' are arranged in an ordered and mapped format, internally implemented using the C++ STL.

Because <tt>config</tt> data is utilized in so many ways and places, it can be difficult to track across the scope of the entire program. Thus, you should document all public functions that take/return <tt>config</tt> objects, specifying content expectations and updating any related entries in the [[ReferenceWML]] wiki pages. In particular, if your function requires a <tt>config</tt> parameter, specify where/how the <tt>config</tt> object 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.

== See also ==

* [[HackingWesnoth]]

[[Category:Development]]