Libxfce4ui api review

[Brian J. Tarricone]

On 2009/07/07 13:40, Nick Schermer wrote:
> 2009/7/7 Brian J. Tarricone<brian at tarricone.org>:
>> Or instead of the enum, could also abuse the GTK_STOCK_DIALOG_* strings.
>>   Personally I like this better because of fewer entry points, and if we want
>> to add other dialog types in the future (no idea what, though, so this might
>> be a silly consideration), it's easy to do without having to add more
>> functions.  Yes, it's a little bit more typing, but not a lot, and IMHO it
>> makes the API cleaner, especially given the _on_screen() variant and the
>> need for 2x the number of functions if you do it the other way.
>>
>> Thoughts?
>
> Personally I think this will only make the api more ugly. Remember
> there is still the possiblity to use the plain xfce_message_dialog
> functions. Splitting the function having a *_on_screen possibility is
> not ideal either. Note that only the info and warning functions take
> the same arguments atm, so the enum won't make it any better too.

Right, I forgot that the error function takes a GError, so no help there.

> Using the pointer parent might look a bit ugly in the first place, but
> if we properly document this and we don't crash on a gchar pointer
> (all arguments are objects so we can do good type checking), I still
> think this is the cleanest solution. Using a pointer is still valid
> (see g_object_unref ^_^, I know for a slightly other reason) and keeps
> the whole thing clean.

g_object_unref() is a special case used to avoid excessive cast macro 
use, since you can expect to want to use it with pretty much any GObject 
subclass.  Stuff that takes a GtkWidget or GtkWindow is a bit more 
restricted in scope (and note that all gtk/gdk functions that have 
parameters for a parent window are strongly typed as taking GtkWindow*). 
  The gpointer arg to g_signal_connect() is there because there's 
nothing GObject-specific about it; you really could pass a pointer to an 
arbitrary struct, as long as it's a descendant of GTypeInterface (even 
if it's not, you still might be able to use it).

Regardless, using a gpointer in this case really doesn't fall under 
either of those cases, because you're using it effectively as an untyped 
(or multi-type) parameter.  For g_object_unref(), everything that you 
pass to it is a GObject.  In your case, you want to be able to pass a 
GdkScreen or a GtkWindow, neither of which are the "same thing"; in fact 
they are very different.  And also the functionality is very different, 
and passing the different things has a different effect: one way says 
"put the window on this screen" and the other says "make the window a 
transient of this other window" (the fact that it also specifies the 
screen is incidental).

Frankly, it's just ugly API, and documenting it well doesn't change that.

What are your objections to the _on_screen() variants, aside from just 
having extra entry points?

	-brian

P.S.  I'm still not totally sold on passing GtkWidget *parent, and using 
gtk_widget_get_toplevel() internally.  The argument of "but calling 
GTK_WINDOW(gtk_widget_get_toplevel()) inline in the function call is too 
much typing and ugly" is best answered by, "then don't use C; use 
another language with a binding that doesn't suck."  But then again, 
this is a library of convenience/extension functions, so I guess it's 
somewhat ok.