Janitorial tasks

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

Inline function removal

• In src/xml/repr.h, there are a bunch of inline functions that are trivial wrappers for SPRepr member functions. Your task, should you choose to accept it: replace all calls to those wrapper functions with direct calls to the wrapped member functions, then remove the unused wrapper functions. (mental) (some patches have been applied from Robert Crosbie)
  • mental says don't touch sp_repr_parent and sp_repr_next (mentioned sp_repr_prev, but it doesn't exist)
  • sp_repr_children (same as above?)
  • sp_repr_unparent (same as above?)
  • sp_repr_ref, sp_repr_unref and sp_repr_set_attr are large conversions needing to be done.
  • sp_repr_document, sp_repr_document_root (do we convert these?)
  • sp_repr_add_child, sp_repr_remove_child and sp_repr_change_order are straight conversions.
• There is a python script that can help for this kind of replacements. Have a look at it: ReplacementScript

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;
};

There are also forward declaration headers that should be used in preference to handcoded forward declarations:

• • XML namespace - #include "xml/xml-forward.h"
  • Extension namespace - #include "extension/extension-forward.h"
  • SPObject related things - #include "forward.h"

(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. Go over those files and put the @file tag in the comment. Also close the comment after the description, and put the author information in a regular comment (see above).
• 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.