Re: GooCanvas items constructors don't follow GObject conventions

[Damon Chaplin <damon karuna eclipse co uk>]

On Tue, 2010-01-26 at 00:32 +0100, Éric ALBER wrote:
> Hello,
> 
> I'm developing a small project using vala. I use GooCanvas through the
> bindings provided by vala.
> I have an annoying problem with GooCanvas item creation methods. For
> example with GooCanvasRect:
> 
> If I want to create an instance of the object, I use
> 
>     GooCanvasItem* item = goo_canvas_rect_new(parent_item, 10, 10,
> 100, 100);
> 
> Now I have a sort of weak reference on the item, I mean I don't need
> to call g_object_unref(item) as parent_item is the owner of this new
> instance.
> 
> 
> But if I create a new instance with these parameters:
> 
>     GooCanvasItem* item = goo_canvas_rect_new(NULL, 10, 10, 100, 100);
> 
> Here I have a strong reference on the item, I need to call
> g_object_unref(item) to be sure the object is freed.
> 
> 
> This means that the ownership of the created instance depends on the
> parameters given to the method. This makes the method unwrappable in
> vala as vala expects the return value of a function to be either weak
> or strong but not dependent on the parameters of the function. More
> specifically, constructors are expected to return strong references.
> In the current state, we cannot subclass any item with the vala
> bindings due to that problem.
> 
> 
> Moreover, In GObject based libraries (GDK, GTK+, etc..), the
> my_object_new(int param1, int param2) methods are just shortcuts to:
> 
>     g_object_new(MY_TYPE_OBJECT, "param1", param1, "param2", param2,
> NULL);
> 
> All specific initialization should take place in a "construct" method,
> not in the my_object_new mehtod.
> 
> 
> So I tried, the following code which should be the same than the
> examples given above:
> 
>     GooCanvasItem* item = g_object_new(GOO_TYPE_CANVAS_RECT, "parent",
> parent_item, "x", 10.0, "y", 10.0, "width", 100.0, "height", 100.0,
> NULL);
>     
> Here I get an instance which properties are initialized with the same
> values than in the first example, but I have a strong reference on it,
> I always have to call g_object_unref on it whether parent_item is NULL
> or not. If parent_item != NULL, ref_count == 2
> 
> If this was the implementation of goo_canvas_rect_new, the method
> could be bound properly in vala.
> I'm willing to help and propose a patch to fix the problem, but I
> would like to know what is your preferred fix:
> 
> 1) Fix goo_canvas_*_new() methods.
> - Pros: API is now clean and similar to all other GObject based
> libraries
> - Cons: All programs/bindings using GooCanvas need to call
> g_object_unref() after creating an instance. Non fixed programs will
> leak memory
> Maybe the version number should be increased to make the API change
> clear to everybody.
> 
> 2) Add another constructor method with a different name like
> goo_canvas_*_new_ref() or goo_canvas_*_create() (which one do you
> prefer?)
> - Pros: All programs using GooCanvas continue to work normally
> - Cons: API is a bit different than other GObject based libraries

I wouldn't want to break API with this so option 1 is out. Maybe if a
GooCanvas 2 is planned one day it could be considered.

My preferred solution would be for the vala bindings to use their own
object creation functions and call g_object_new() as you did above.

Or you could just not wrap the C creation functions and use
g_object_new() directly.

Damon