CodingStyle

From Gnash Project Wiki

Jump to: navigation, search

Contents

C++ considerations

General

  • Do not reinvent the wheel. Use the C++ standard library (supplemented with boost libraries) as much as possible.
  • Feel free to use C++'s features to the fullest extent, including inheritance, templates and exceptions. Boost may be used with the following restrictions:
    • The highest required version of Boost must be 1.34.
    • Header-only libraries may be used without restriction, but the only compiled Boost library generally usable is Boost.Thread. (Boost.Date-Time and Boost.Serialization are used in parts of Gnash code, but must remain optional). Requests to allow the use of more compiled libraries can be discussed!
  • Write portable code. For instance, don't rely on pointers being a specific size - instead, use ptrdiff_t. Functions from libc are acceptable if they are part of standard C++ (in the std namespace).
  • Never include a using directive in a header. Avoid doing so at file scope. Generally it is best to qualify names fully. If a using directive is particularly helpful, apply it at function scope.
  • Do not write:
typedef struct _a {} A;
typedef enum { A, B } B;
typedef struct _C {} C; // This also uses a reserved name _C, which causes undefined behaviour.

In C++ the following definitions are correct:

struct A {};
enum B { A, B };
  • Document your code (especially public interfaces) using Doxygen.
  • Write tests for (your) code and run them prior to committing to trunk.
  • Please fix the warnings your compiler generates from your code. Compiler warnings can be a valuable debugging tool, and if you leave warnings unattended, people will start to ignore the warnings their own code generates.
  • printf(), sprintf(), scanf() and sscanf() are banned.
  • varargs are banned.

Containers

  • Please prefer C++ containers, such as std::vector, over C-style arrays. For most purposes, vector will be the fastest C++ container.
  • Use C++ STL algorithms, such as find, for_each et cetera, in favor of writing manual loops. If you do not, you will miss out on optimization opportunities, make your code harder to read and more error-prone.
  • Prefer container::empty() over container::size() == 0 to test whether or not the container is empty (including std::string!). For some container types, size() is a linear (or worse) algorithm, whereas empty() is (almost) always a constant time algorithm. For most containers the speed difference is negligible, but it is still bad style.

Allocating memory

  • Whenever possible, wrap your memory buffer in a scoped pointer such as std::auto_ptr, boost::scoped_ptr or boost::scoped_array so you do not have to worry about deallocation.
  • Avoid heap allocation whenever possible. When it is necessary, allocate heap memory with operator new or operator new[].
  • Never use malloc or other C allocation functions. External C libraries generally provide their own alloc/free functions for use with their own types.
  • Gnash uses a GC for ActionScript objects for performance reasons. GC-managed objects (e.g. as_object, character) must be allocated with new and not deleted.
  • Never mix operator new and delete [] or vice versa. Memory allocated with operator new must be freed with delete and memory allocated with operator new[] must be freed with delete [].

Miscellaneous

  • Prefer prefix incrementation ++i to postfix i++. The postfix operator returns a copy, requiring creation of a temporary, and although most compilers can optimize this away it is bad style. Do not assign and increment in a single statement:

*p++ = 0;

Instead, please separate the two.

*p = 0;
++p;
  • Use an anonymous namespace in preference to the file-scope static keyword for functions:
namespace {
    int doSomething(const std::string& s) { return s.size(); }
}
  • Do not forget to mark free functions defined in headers with the keyword "inline". Failure to do so can easily result in duplicate definitions and a failure at link time.

Strings

  • Always use std::string to represent character strings. If you are using a C API, you can use the string::c_str() method to obtain a C style string.
  • You do not have to (and you should not) initialize a string to an empty literal (or something along those lines). string's default constructor will take care of that. Use clear() to reset the string to its default empty state.

Forward declarations

  • Use forward declarations whenever possible. This will help reduce compiling time.

Casting

  • Always use C++ style casts in favor of C style casts. You may use RTTI casts (dynamic_cast), but avoid them where overriding a virtual function is an alternative. Don't forget that a dynamic_cast of a reference can throw an exception.

Classes

Use the class-key "class", not "struct" for classes. The class-key "struct" should only be used for collections comprising only public non-static data members:

 class A {
 public:
     A() : i(0) {}
     void f();
 private:
     int _i;
 };
 struct B {
     int i;
     double d;
     float f;
 };

Constructors

  • Initialize class variables in the constructor's initialization list.

Destructors

  • If your class has virtuals, then you must also declare a virtual destructor. Omitting this will probably cause memory leaks later!
  • If your class doesn't have virtuals and your member variables are destroyed when they go out of scope (e.g. they are simple data types and/or scoped pointers), then you don't have to declare and/or implement a destructor.

Member functions

  • if you have a member function which you can implement in a single line, you may implement it in the class declaration, but this is not required. It will then be inlined by the compiler.

Assignment and copy constructors

  • All classes should have a copy constructor and assignment operator or inherit from boost::noncopyable. This prevents objects being copied using a compiler-generated constructor.
  • If just one of the two is needed, declare the other as private and do not define it. This will prevent assignment / copying at compile time.

Generic code

  • Do not use void* to make your code generic. There are better, type safe constructs for this in C++, such as templates and inheritance. In fact, there should be no need to use void* at all, unless you are dealing with an external C API.

Headers

  • Don't use the standard C headers, use their C++ equivalent, i.e. <cstdio> instead of <stdio.h>. However, in the majority of cases you should prefer C++ libraries, such as <iostream>.
  • Always use a header guard:
#ifndef GNASH_HEADER_NAME_H
#define GNASH_HEADER_NAME_H

// Header content

#endif
  • __HEADER_H__ or GNASH_HEADER__H are not permissible header guards. All names containing a double underscore are reserved for the C++ implementation.
  • _HEADER_H_ is not a permissible header guard. All names starting with _ followed by an upper-case letter are reserved to the C++ implementation, as are all names beginning with _ in the global namespace.

Macros

  • Do not use macros to generate code. Use inline functions and templates instead.
  • The only exception to this is the Boost.Preprocessor wizardry for logging and other cases where a variadic function is needed.

Constants

  • Do not use preprocessor directives to define constants. Use a const type instead. The compiler is capable of substituting a variable with a constant when it is faster to do so.

Other preprocessor directives

  • Generally, try to avoid cluttering your code with preprocessor directives to avoid #ifdef hell to the point where your code becomes illegible and unmaintainable. Try to abstract away (sub)system specific code using inheritance and templates.
  • #ifdefs for operating-system compatibility should be restricted to libbase if humanly possible. Please use the libbase compatibility headers for system functions (read(), write() etc) when one is available, and try to make such a header if one is not.
  • Disable temporary code with #if 0 (do not comment it out). Add a comment inside the #if 0 block explaining why the code is disabled.
  • Use DSOEXPORT for classes, method and functions that are used across DSOs. This will allow the use of visibility features for modern compilers. This is essential for exceptions that are thrown across DSOs.

Formatting

For existing code

  • Please use the (predominant) style in the file you are editing. If the style used throughout the file is inconsistent, use the style rules for new code and feel free to change the rest of the file to match the new style as well.

For new code

Indentation

C++

  • Current consensus: four spaces, no tabs.
  • Limit line length to 80 characters.

M4 and (BA)Sh

  • You must indent with two spaces.

Loops

for (init; condition; incr) {
    ...
}

while (condition) {
    ...
}

do {
    ...
} while (...);

If-else

if (condition1) {
    ...
} else if (condition2) {
    ...
} else {
    ...
}

Avoid ambiguity in condition statements by avoiding...

 if (condition1 && condition2 || condition3) {

Instead, use more parentheses than strictly necessary.

 if ((condition1 && condition2) || condition3) {

Naming convention

  • Types (classes, typedefs, enums): class MyClass, typedef std::vector<int> Numbers, enum VideoCodec.
  • A typedef name should describe the purpose of the type, not what it is:
    • Bad: typedef std::vector<int> IntVector. Can't sensibly be changed to alias a std::list or std::set.
    • Good: typedef std::vector<int> Characters. A type for holding characters. CharacterList is also just about appropriate, as list has a meaning in normal English, but is still potentially confusing.
  • Class member functions (methods): memberFunc().
  • Class (private/protected) data fields: _data_member or _dataMember.
  • Static class member functions: staticMember().
  • Class member implementation: state the return type on its own line.
  • An example:

MyClass.h:

namespace gnash
{

class MyClass
{
    public:
        void memberFunc();
        static void staticMember();

    private:
        int _my_variable;
};

}

And in the implementation in MyClass.cpp:

void
MyClass::memberFunc()
{
    _my_variable = 88;
}

void
MyClass::staticMember()
{
}