Difference between revisions of "CodingStandards"

From The Battle for Wesnoth Wiki
(See Also)
m (Write Exception-Safe Code: - scoped_sdl_surface and shared_sdl_surface are now obsoleted by the "surface" class)
Line 36: Line 36:
 
== Write Exception-Safe Code ==
 
== 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
+
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).
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,
Line 45: Line 43:
 
   SDL_Surface* image = IMG_Load("image.bmp");
 
   SDL_Surface* image = IMG_Load("image.bmp");
 
   ...some code, which uses 'image'...
 
   ...some code, which uses 'image'...
   SDL_[[FreeSurface]](image);
+
   SDL_FreeSurface(image);
 
   }
 
   }
  
is bad, because the code may throw an exception, and 'image' will never be freed. Instead, use wrapper objects which
+
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.
free the object in their destructor.
 
  
For SDL_Surface objects you can use scoped_sdl_surface. You can also use smart_sdl_surface for surfaces that need to
+
For SDL_Surface objects, use the <tt>surface</tt> class. So you could rewrite the above code,
be
 
copied/stay persistent. So you could rewrite the above code,
 
  
 
   {
 
   {
   scoped_sdl_surface image(IMG_Load("image.bmp"));
+
   surface image(IMG_Load("image.bmp"));
   ...some code, which uses 'image' much like it was a SDL_Surface*...
+
   ...some code, which uses 'image'...
 
   } ''the image is automatically freed here when 'image' is destroyed
 
   } ''the image is automatically freed here when 'image' is destroyed
  

Revision as of 16:53, 24 October 2005

Wesnoth uses modern/advanced C++ that is portable to Visual C++ 6 and GNU G++ 3.0+

End Non-Public Members of Classes 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.

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 Const

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.

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.

Respect for loop scoping of different platforms

In the code,

 for(int i = 0; i != 100; ++i) {...}

the variable 'i' is scoped within the for loop according to ISO/ANSI C++ and GNU G++. However it is scoped within the surrounding scope according to Visual C++ 6.

This means that the code,

 for(int i = 0; i != 100; ++i) {}
 for(int i = 0; i != 100; ++i) {}

is illegal on VC++6, because i is defined twice, although it is legal according to the standard, and GNU G++.

On VC++6, the legal way to write it would be,

 for(int i = 0; i != 100; ++i) {}
 for(i = 0; i != 100; ++i) {}

But this is illegal according to the standard, because 'i' is not defined in the second loop. The correct way to write this code to conform to the standard and work on all platforms is to simply abandon declaring variables in the initialization statement of a for loop when the variable must be reused in the same scope,

 int i;
 for(i = 0; i != 100; ++i) {}
 for(i = 0; i != 100; ++i) {}

Use the function templates minimum and maximum

Standard C++ offers the function templates min and max to find the minimum and maximum of two values on which operator < is defined. Unfortunately, many hoops must be leapt through to get this working on VC++. So, we do not use standard min and max. Instead, we use minimum and maximum, defined in utils.hpp.

Usage is fairly natural:

 int i = minimum(x,y);

Note that in the above example, if x is an unsigned integer, and y is a signed integer, VC++ will have problems. You must explicitly specify the version of minimum being called in such cases:

 int i = minimum<int>(x,y);

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 >

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.