GNOME Bugzilla – Bug 732746
Interface properties shall be instance data
Last modified: 2014-07-21 13:54:31 UTC
Created attachment 279926 [details] Test case A patch by José Alburquerque has made it possible to make custom classes that derive from interfaces with properties, such as class CustomAction : public Gio::Action, public Glib::Object This is discussed in bug 697229 comments 13-16, 18-19, 21-25. That patch is basically good and welcome, but some of my questions were never really sorted out. I think some details could and should be improved. I repeat here some of the questions I asked in bug 697229 comment 21, together with new comments. 1. Would it be a good idea to add g_param_value_set_default(iface_props[p], g_value) in Interface::Interface(const Interface_Class& interface_class) and Class::custom_class_init_function(void* g_class, void* class_data)? g_param_value_set_default() was added to the Interface constructor, but not to Class::custom_class_init_function(). That's easily fixed. 3. The Class::properties_type vector that holds the values of the overridden properties of implemented interfaces is attached to the custom_type with g_type_set_qdata(). Those values are thus class data. They shall be instance data. Each instance of the custom class shall have its own set of property values. The comments in bug 697229 indicate that José does not agree with me. I claim that everything about a property (name, type, default value, etc.) except its value is class data. But each instance of the class shall have its own copy of the property's value. I attach a test case that shows how a custom implementation of Gio::Action (with read-only properties) and Gtk::Orientable (with a read-write property) work. 4. Don't we need protected access methods to the overridden properties? Accesses through property_xxx() methods generated by _WRAP_PROPERTY are probably enough for read-write props, but what about read-only and write- only props? The class that implements the interface must be able to write a read-only prop, and read a write-only prop. Comments in the test case show that this is more complicated than I thought when I first made this comment. Perhaps a reasonable "solution" is that a custom class that need read-only or write-only properties from an implemented interface must provide set-methods for write-only props and get-methods for read-only props, and private data members to store the values.
Created attachment 279927 [details] Output from the test case The output shows: 1. The test case fails to set a value to the read-only Gio::Action:: property_name(). "So what?" you might say. "Of course you shall not be able to write to a read-only property." But a read-only property is like a private data member with a get-method but no set-sethod. The object that owns the property (in this case a CustomAction) shall be able to both read and write it. 2. When a value is set to Gtk::Orientable::property_orientation(), the value pertains to both CustomAction objects. That's because the value is class data instead of instance data. That's not how properties shall behave. What shall be done? I suggest that items 1 and 3 in comment 0 shall be fixed. Drop item 4. It's probably no big issue and there is an easy workaround (mentioned after item 4). The patches in bug 731484 comment 0 and bug 731484 comment 2 also deserve mentioning here.
Created attachment 280075 [details] [review] patch: Make custom interface properties instance data This patch fixes items 1 and 3. I'll let it rest here for a while before I push it. It not obvious how to best fix this. Hopefully someone will have a look at it.
Created attachment 280278 [details] Test case 2, source code and results A tar file with the files testinterfaceprops2.cc Source code testinterfaceprops2-1.txt Results without the patch in comment 2 testinterfaceprops2-2.txt Results with the patch This is a better test case. It tests both how default values are handled, and if the properties are instance data.
I have pushed the patch in comment 2, with minor changes in comments. https://git.gnome.org/browse/glibmm/commit/?id=08da2fbbb763b81fd6f1e3074aaaf83694c012ed I wanted to give José a chance to comment, since he has fixed most of the interface property stuff in custom classes. Then I happened to see bug 686909 comment 9, and realized that he will not have the time to do so. Understandable but unfortunate.