Re: [Inkscape-devel] GObject removal branch

First of all, thanks for your comments :) .

1. The code seems wrong in a several places. For example, the constructor of SPObject retains the manual constructor calls and its destructor retains the manual destructor calls. Member initialization is already done before the constructor is even entered, and member destruction happens automatically after the destructor returns. I'm fairly sure this will lead to problems at runtime.

I didn't see them, I thought I fixed that everywhere...

2. A lot of the constructor code should be changed to member initializers. See e.g. the constructor of SPItem.

I agree.

4. "#pragma once" is not used in existing Inkscape code. For consistency you should use standard include guards.

Okay.

5. singleton.h should be merged into sp-factory.h. Singletons should be avoided where possible, so a separate singleton header is not a good idea. Boost doesn't have a singleton header for the same reason.

I just did that as I noticed there are some other classes that use a singleton pattern as well, e. g. DocumentMetadata.

3. Class names such as CSomething are reminiscent of MFC. The prefixes should be removed and the classes put in namespace Inkscape::SVG (since they represent objects in the SVG).
4. Method and variable names are not consistent with our coding style, though this could be addressed later since it's a leftover of previous GObject code.

http://staging.inkscape.org/en/develop/coding-style/ http://inkscape.org/doc/coding_style.php (old website) I'll have a look.

7. The virtual methods should be made protected, since they are not intended for use outside of SP node implementations.

Okay.

Regards, Markus