On Sat, May 24, 2014, at 03:43 AM, Tomasz Boczkowski wrote:

Resending: original message was send accidentally off-list.

>> A safe way to mitigate many of the C vs C++ issues with Gtkmm is to keep a proper logical separation between Gtkmm UI objects and the functionality they trigger or state they manifest. The bulk of code ends up in plain C++ classes, while the Gtkmm widgets merely provide a thin layer to expose

>> in the 'proper' manner. That might including moving certain functionality out of the widget classes themselves.

I see your point. Could you please name some inkscape UI classes that preserve the separation?

I'm going through the codebase to see what examples we might want to follow. There are a some that might need to be taken a step further, though.

I would like to go on with SPColorSelector refactoring. In Inkscape color can be selected using one of two widgets: sp-color-wheel-selector, sp-color-icc-selector. They share common functionality in sp-color-selector. Currently there are six structures consisting of both Gobjects and c++ classes.

I made a simplified diagram, listing possible options for refactoring [1].

1) ColorSelector widgets contain a reference to their state (ColorSelector). It is accessible via getColorSelector() method. Optional ColorSelectorWidget abstract class provides the method. Parent UI component would initialize color selector widgets in ctor. Reference to widget would be kept in ctor only. Parent class would contain a pointer to widget state, returned by getColorSelector().

2) Double inheritance. ColorSelector provides widget state. Concrete implementations inherit ColorSelector and aproppriate Gtk::Widget.

3) Full wrapper. Color selector widgets contain reference to their state and have full set of thin wrapper methods. They only call corresponding methods in ColorSelector.

Which is the best? What is the suggested naming and namespacing of mentioned classes?

Which of the options fits the separation between gtkmm and functionality best?

The diagrams seem quite helpful. However, in this instance it can also be helpful to also start with some test code, as per standard TDD approaches.

The first question that pops into my mind is "why?" for the getColorSelector() method. Once things are set up that might not come into play. Another factor would be to try to avoid multiple inheritance. In this usage, needing to inherit from a Gtkmm widget plus something different just doesn't quite seem to be clean and logical (aka might not meet 'coherence' criteria for good OO design).

Gtk+ does have the Action object that has a few aspects we might care about. It is an abstract instance, however there are factory methods to get an appropriate widget as needed. This allows one to place a GtkAction in a menu or a toolbar or another contain and have it manifest as a proper widget as needed. In the case of placement in a toolbar, a button widget is normally created. However if actions do not fit in the visible area of the toolbar, they are placed in an overflow menu and manifest as menu item widgets.

With GtkAction the class itself contains factory methods to create needed widget instances. For a clean separation we might want to have either a factory class instance or a set of factory functions to create the needed widgets. The calls would return just a Gtk::Widget * pointer, and thus provide properly encapsulated instances. As a consumer we don't know what they do, just that they can be placed in a UI container. Or rather than a plain Gtk::Widget* we might return Gtk::MenuItem* and Gtk::ToolItem.

The code in the factory method would take in an instance of ColorSelector, wire it up to one or more Gtk::Widget subclasses with signal handlers, etc. then return a final parent Gtk::Widget (MenuItem or ToolItem as appropriate) to be placed in a UI. So we might have factory functions for

Gtk::MenuItem *createMenuItem(ColorSelector &selector)

Gtk::ToolItem *createToolItem(ColorSelector &selector)

However we definitely would need at least a base dialog/panel factory:

Gtk::Widget *createPanel(ColorSelector &selector)

All this, of course, is just some initial pondering to get us going on some directions to look at. Actually writing some test code can help to quickly pin down what details will be needed or not.

Then we get to one more problem your UML diagram pointed out. The term "ColorSelector" seems to be very UI centric. That name in and of itself might cloud understanding of a cleaner separation. If we look for some name that more properly indicates the *what* of what it is rather than the *how* it's been implemented in the UI, that name can help us move forward. We might consider "ColorSelection", but that second half is now misleading (as we don't use it to collect up a set of things the user wants to act on). "Color" is too generic as is "ColorData", but "MutableColor" starts to clarify it a bit.

Add in our Panel class for abstract UI container and we might get something like

Panel *createColorSelectorPanel(MutableColor &color);

Jon A. Cruz

jon@...18...