Refactoring C to C++ Part 1

It turns out that a recent Inkscape source change is a good example for showing some of the process of conversion from C to C++ of a GTK+ type. In doing some recent usability changes, I'd done a bit of a cleanup on 'C++ifying' the Inkscape SPCtrlLine type. Trying to keep our source revision history clear and useful, this one cleanup pass went in as a separate change (revision 11321). This also makes it easy to look at for guidance.

A good starting point is to look at the changes to the main header file itself: sp-ctrlline.h.

First is a simple change to a standard GTK+ macro definition. Yes, in general macros are evil, but the few macros listed at the start of the header are following GTK+ conventions.

21    #define SP_TYPE_CTRLLINE (sp_ctrlline_get_type ())
   23 #define SP_TYPE_CTRLLINE (SPCtrlLine::getType())

• The "SP" prefixing is legacy naming that we will ignore for now.
• In general this seems like a minor change, with only subtle formatting differences, but there is more to it than that.
• Instead of invoking a single function with a long name, it now invokes a static method on a class.
• The method being called is now merely "getType()" (and thus is template-friendly).

One important point to keep in mind is that in C++, a struct is just a class that defaults to public:. So once we're in C++-land, just think of "struct" as a rough synonym for "class".

Then the main change in the header involves moving a set of simple C functions to instead be class methods:

33  GType sp_ctrlline_get_type (void);
34 
35  void sp_ctrlline_set_rgba32 (SPCtrlLine *cl, guint32 rgba);
36  void sp_ctrlline_set_coords (SPCtrlLine *cl, gdouble x0, gdouble y0, gdouble x1, gdouble y1);
37  void sp_ctrlline_set_coords (SPCtrlLine *cl, const Geom::Point start, const Geom::Point end);

• Since sp_ctrlline_get_type() does not have a pointer to an instance, this will be a static method
• Since the others start with SPCtrlLine *cl instance pointers, these will become normal methods.
• The prefix "sp_ctrlline_" dissappears as a natural part of moving into a class.
• The explicit instance pointers (SPCtrlLine *cl) dissappear and are replaced by the implicit "this" pointer of C++ member functions (aka "methods").
• To avoid making unnecessary copies of the start and end parameters on sp_ctrlline_set_coords, we change it to pass constant references instead.
• Since C++ references are easiest to understand when read left-to-right, we move the 'const' to be just before the & of the reference.

28    static GType getType();
30    void setRgba32(guint32 rgba);
32    void setCoords(gdouble x0, gdouble y0, gdouble x1, gdouble y1);
34    void setCoords(Geom::Point const &start, Geom::Point const &end);

Moving on now to the sp-strlline.cpp file, there are a few things to note. One is switching from static methods to using an unnamed (or anonymous) namespace. That could have allowed us to drop the "sp_ctrlline_" prefix, but that step was skipped for the moment. We do, however, want to fix casts as we go, such as

49        (GClassInitFunc) sp_ctrlline_class_init, 
   51     reinterpret_cast<GClassInitFunc>(sp_ctrlline_class_init),

Inside of the class_init function around lines 63-72/66-72 there is a simplification due to inheritance. There is no need to create object_class and item_class pointers from the passed in SOCtrlLineClass *klass pointer. The members of the parent types are visible, so we can just use klass directly, such as for

klass->destroy = sp_ctrlline_destroy;

Another handy aspect to turning stand-alone C functions in to C++ methods is that we get compile-type checks and safety and can drop run-time checks, such as at the beginning of the new SPCtrlLine::setRgba32() method:

154    g_return_if_fail (cl != NULL);
155    g_return_if_fail (SP_IS_CTRLLINE (cl));

The checks at lines 171-172 are similarly dropped.

Once we get to the body of the method, there are a few interesting points to be seen:

157        if (rgba != cl->rgba) {
158            SPCanvasItem *item;
159            cl->rgba = rgba;
160            item = SP_CANVAS_ITEM (cl);
161            item->canvas->requestRedraw((int)item->x1, (int)item->y1, (int)item->x2, (int)item->y2);
    155    if (rgba != this->rgba) {
    156        this->rgba = rgba;
    157        canvas->requestRedraw(x1, y1, x2, y2);

• At new line 155 since a parameter has the same name as a member, we use "this->" to be able to access the member.
• There is no need for the casting macro SP_CANVAS_ITEM from line 160, since a subclass has all the superclass accessible.
• Since canvas, x1, y1, x2 and y2 are all members and we are now a member function, use of cl-> and item-> can be dropped.
• Since canvas is a member and we are in a member function, we can use it directly in new line 157.
• C-style casts, and casting in general, are enemies. By dropping the casts to (int), we let the code get simpler, gain the ability to leverage from overloading, and get errors more visible.

Moving on down into gradient-drag.cpp, there is a very important shift in though/approach for pointers. Looking at line 1579/1578 we see a difference in type:

1579         SPCanvasItem *line = sp_canvas_item_new(sp_desktop_controls(this->desktop),
1580                                                                  SP_TYPE_CTRLLINE, NULL);
     1578    SPCtrlLine *line = SP_CTRLLINE(sp_canvas_item_new(sp_desktop_controls(this->desktop), SP_TYPE_CTRLLINE, NULL));

Instead of holding a pointer to the more generic parent class SPCanvasItem, we hold and use a more specific pointer to the sublcass SPCtrlLine.

With GTK+ in C, holding the more generic type is common, and results in, among other things, excessive use of the type check and type casting macros (such as SP_CTRLLINE()). Aside from any performance slowdown they introduce, they hide things, block overriding, and sacrifice compile-time safety for run-time checks. It is far better to have incorrect code that will result in the compiler rejecting it upfront rather than code that will fail at runtime (but only when a user trips over the specific code path in question).

Similar fixes can be seen in the changes to line-geometry.cpp and elsewhere. In pen-context.h, seltrans.h, text-context.h, and node.h the type of the pertinent members have also been changed from the parent class SPCanvasItem to the more specific subclass SPCtrlLine.

In closing, reviewing the entire change with thoughts as to why different things were done can be quite useful. At some point soon I'll be following up with some more examples, along with some summaries of key points to follow and keep in mind. Additionally, this change did not really touch on any conversion from plain GTK+ over to Gtkmm (the C++ wrapper library for GKT+). Subsequent entries will also touch on those.