Re: [GnomeMeeting-devel-list] [PATCH] druid reorganisation



OK after reading it, it only moves a bit of code around here and there,
but there are 2 things that must absolutely change :

- your naming of the functions is worse than what it was. 
If you create the druid with gm_druid_new, then all your callbacks and
functions related to the gm_druid should be prefixed by gm_druid.

- your callbacks are internal callbacks for the gm_druid object, so
that's perfectly rational to pass the internal structure as parameter.
In the case where your callbacks would be shared by several parts of the
code, it is better to keep the pointer parameter for other uses to pass
real parameters and not constant ones. However, I would prefix the
callbacks by gm_druid.

- You have split a callback in many pieces, the advantage is more than
discussable, but if you like it, that's ok as long as I don't dislike
it. But don't forget documentation !!

- Your static functions should take the GtkWidget *druid as argument,
not the private data, that's a matter of consistency. Perhaps we should
add in misc.cpp a function named gm_get_object_from_widget.

That's all for my remarks.

Le ven, 07/05/2004 à 15:01 +0200, PUYDT Julien a écrit :
> Hi,
> 
> this patch is a reorganisation of the druid. Hopefully it is more
> readable. At least its interface with the rest of gnomemeeting is
> cleaner.
> 
> The best way to review is certainly: read the diff for everything but
> src/druid.cpp, and study the full patched src/druid.cpp.
> 
> I'm not entirely satisfied with that patch yet, and would like some
> input on it.
> 
> Snark
> _______________________________________________
> Gnomemeeting-devel-list mailing list
> Gnomemeeting-devel-list gnome org
> http://mail.gnome.org/mailman/listinfo/gnomemeeting-devel-list

Attachment: signature.asc
Description: Ceci est une partie de message =?ISO-8859-1?Q?num=E9riquement?= =?ISO-8859-1?Q?_sign=E9e?=



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