Re: GooCanvas items constructors don't follow GObject conventions



2010/1/26 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


 
OK, I understand that breaking the API is not a good solution.

But using g_object_new is not a valid option for vala bindings either.
In fact vala bindings are not libraries like goocanvasmm for C++.
Vala generates C code which uses directly the C implementation of the libraries.
So if a function is not available in the C library, it will not be available in the vala binding either.
Even writing a simple function that calls g_object_new is not possible.
That's why these functions should be added to goocanvas directly.

Regards,

Eric


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]