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.
  • Using a Doxygen comment with @file at the top should no longer be the normal case. The comments need to document individual classes, subsystems, etc., and not be focused on file structure. Doxygen itself normally will address file-specific needs.

/*
 * 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:
  • 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.)
  • 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. File-centric Doxygen comments should normally be avoided, thus these should be moved elsewhere. Thus the file should start a regular comment with the author and copyright information.
  • If there is an initial Doxygen comment, check if it is simply duplicating information in other Doxygen comments (especially in a corresponding .h file when looking at a .cpp file). Such can be safely removed.
  • If a comment does appear to be file-centric, ask for some quick guidance to see if it actually applies to a subsystem and not a file, it it is redundant, etc. to get it cleaned up.
• In general, beginning files with a @file Doxygen comment is an outdated practice, and should be cleaned up.
• Some documentation is useless, for example "constructor" or "destructor". Such comments mark the entity as documented, when in fact it's not. Remove them.

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.