Janitorial tasks

Before embarking on any of those tasks, be sure to contact the developers on the mailing list.

Improving headers for compilation speed

When a header file changes, any file that includes it is recompiled. This becomes a problem when a given header file is included by other header files, as it leads to a vast header include tree, wherein a small change to a seemingly minor header file causes massive rebuilds of much of the codebase.

Fortunately for us, there are recognised techniques to mitigate this! Here's what to do:

• Find a header that includes several other headers. For example, the header "bar.h" might have a class Bar with a member of type Foo*, thus it includes "foo.h" to get the definition of that type. You can prune this by using a 'forward declaration', whenever the Foo entity is not itself used, only pointers and/or references to it:

//#include "foo.h"  /* <-- kill the header include! */
class Foo;          /* <-- replace with a forward declaration */
class Bar {
    Foo* _foo;
};

Although there are still forward declaration headers in the source tree, those should be avoided in favor of individual forward declarations in .h files as needed.

(BPF) The C++ FAQ Sheet explains how to code forward declarations (classes that both need to know about each other); To fully understand why, you should study the Pimpl idiom.

Source formatting

• Source files should use four spaces as indentation and no tabs. Trailing whitespace should be removed.
• The comment at the top of each file should have the following format. The author information is in a regular multiline comment. Author emails can be obfuscated, but should be real addresses.

/**
 * @file
 * Logarithmic time travelling salesman solver
 *//*
 * Authors:
 *   J. God Hacker <ihatepizza@gurus.org>
 *   Ellen Epic <epicwin at email dot com>
 *
 * Copyright (C) 2006-2008 Authors
 * Released under GNU GPL, read the file 'COPYING' for more information
 */

• Every source file should have the following Emacs local variable block and Vim modeline at the end:

/*
  Local Variables:
  mode:c++
  c-file-style:"stroustrup"
  c-file-offsets:((innamespace . 0)(inline-open . 0)(case-label . +))
  indent-tabs-mode:nil
  fill-column:99
  End:
*/
// vim: filetype=cpp:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:fileencoding=utf-8:textwidth=99 :

• Include style. In-tree includes should use quotes, while system headers should use angle brackets. An exception is 2Geom, which should use angle brackets, though it is local (we are preparing for it to become a standalone library one day). Includes in each group should be sorted alphabetically. The path should be relative to the src/ directory. If there is a config.h include, it should go at the top. Here is an example:

#ifdef HAVE_CONFIG_H
# include "config.h"
#endif

#include <cairo.h>
#include <cstdio>
#include <glib.h>
#include <math.h>

#include "display/cairo-utils.h"
#include "document.h"
#include "sp-use.h"
#include "xml/node.h"

• Order of the file. Each file should contain the following, in precisely that order:
  • @file comment, which marks the file as containing documented entities. You can skip it for files that contain no documentation.
  • Copyright comment, as above.
  • Include guard (only for header files)
  • System includes
  • Local includes
  • Forward declarations (note: although using an explicit forward declaration file used to be recommended, it no longer is. Using one had been found to be inefficient and error-prone. note on this note: lib2geom devs like 2geom/forward.h, so keep using it there! )
  • Class declarations
  • Function declarations
  • Global variable declarations (note: global variables should be avoided)
  • End of include guard
  • Emacs local variables block (see above)
  • Vim modeline (see above)

Documentation

• Many files begin with a Doxygen comment that doesn't contain the @file tag. The result is that the comment doesn't document the file, but the first entity declared in the file, usually the Inkscape namespace. Add a @file directive to them.
• Some documentation is useless, for example "constructor" or "destructor". Such comments mark the entity as documented, when in fact it's not. Remove them.
• When the @brief tag is skipped, Doxygen will use the first sentence (ending with a dot) as the brief description. An alternative is to put the description in a single-line comment. These two techniques can be used to reduce the number of directives. In the example below, all three functions will have the same documentation. The second case depends on the variable JAVADOC_AUTOBRIEF being set to true:

/// Something useful
/** This function does something very useful.
 * Here is its more detailed, longer description. */
void useful_function_one();

/** Something useful.
 * This function does something very useful.
 * Here is its more detailed, longer description. */
void useful_function_two();

/** @brief Something useful.
 * This function does something very useful.
 * Here is its more detailed, longer description. */
void useful_function_three();

Coding style

• Replace C-style casts with the appropriate C++ casts. You can compile with -Wold-style-casts to find them easily.
  • static_cast when the conversion is obvious, for example a floating point to integer type.
  • const_cast if the only difference between the types are const qualifiers.
  • reinterpret_cast if the conversion does not compile with static_cast, for example pointer to integer.
  • dynamic_cast for downcasting to derived class type.

Elimination of old utest tests

It should be double-checked that the old utest tests (also see TestSuite-blueprint are indeed all obsolete. If there happen to be any left that are not obsolete they should of course be converted to the CxxTest framework (if you don't feel up to it, ask me). Finally, the obsolete files should be removed from the repository and Makefiles, making sure that nothing breaks in the process.