Re: Proposed changes in libgda

[Rodrigo Moya <rodrigo linuxave net>]

Hi Derek!

> After (nearly) having finished the conversion from gtk objects to glib
> objects in libgda, I have collected a list of changes I would propose to
> libgda to make the code even cleaner, better organized and better
> understandable than it is now (and maybe even better usable):
> 
> * gda-init should be a part of the client library, because the server
> library has it's own gda-server-init, and it should have its own
> gda-init.h
> 
no, it will be needed for GDA-non-client applications, like the report part.

> * gda-config should be a part of the client library, too, because it's
> purely client-sided
>
the same as with gda_init applies here.
 
> * the names of the macros for the objects could be standardized (e.g. there
> are now GDA_IS_XML_FILE, but IS_GDA_THREAD
>
yes, this is something I've been forgetting systematically. It would be great
to provide a shell script which makes the change, so that client apps
just have to run it to conform with the new names.
 
> * every object should have a gda_xxx_free method which only frees the
> object, and the destructor (gda_xxx_destroy/gda_xxx_finalize) should be
> private and do all the actual work
>
yes, this is also something actually broken. Some objects do the _destroy
stuff properly but others not, so yes, it's ok.
 
> * gda_connection_new shouldn't take a parameter, the orb needs not be
> passed from outside this function; this change would break existing apps
> but would make usage of libgda easier, as normally you wouldn't have to
> deal with orbit/oaf as a libgda user
>
yes, it's also ok. This change would need manual changes in client apps,
since sometimes it's called with gda_connection_new(orb) and sometimes
with gda_connection_new(gda_corba_get_orb()).
 
> * the actual definition of the _Gda_* structures could be in the .c file
> instead of the .h file and thus be _real_ private; this would enforce users
> of libgda to not use the internals of this structure and therefore give us
> the freedom to change them
>
instead of this, I would do what is being done in some other GNOME libs: have
the _Gda_* structures just contain the base class object and a
_Gda_*Private pointer, which is then defined in the .c file.
I think we should leave the _Gda_* structures in the .h files, first because
of consistency with the rest (GNOME, GTK, GNOME-DB, etc), and second
because people would know what the base class of every object is.
An example of this which I've started to do on gnome-db, is on
gnome-db/lib/gnome-db-control*
 
> * as discussed before we could use AM_PATH_* for orbit, oaf, and gconf in
> configure.in
>
yes.
 
> * Makefile.am's could be cleaned up a bit. GDA_CLIENT_CFLAGS/LIBS and
> GDA_PROVIDER_CFLAGS/LIBS should not be used for the library itself, but
> rather for the client apps and the providers. I say this because e.g.
> gda-server does not depend on libxml and GConf, and gda-client does not
> depend on libxml, GConf and oaf (of course this could change).
>
I'm not sure about this. Both the client and server libs use the gda-common
lib, which in turn must link with libxml and GConf
 
> * gda-config should call gconf-config, oaf-config, orbit-config, xml-config
> and glib-config and add the output of them to it's own output (like
> oaf-config calls orbit-config in the current version).
>
yes, also ok
 
> * the macros directory should be removed IMHO, and a non-gnome autogen.sh
> should be used
>
yes
 
> * what would you think about a private header file in each directory
> (like common.h, defines.h or gda-clientP.h etc.) in which we include
> the things that are now included in all .c files (config.h, the NLS
> stuff...)? We would _not_ install that header, and it would be only
> included in our .c files, not in any other headers.
>
yes
 
> * what would you think about creating a gda.m4 for all those projects
> that use gda?
>
wow! perfect!
 
> * Gda_Recordsets defines signals, but they are never emitted. Maybe
> they could be removed.
>
yes, I'll have a look at it, because it is in its current state due to the
fact that it's based on GtkAdjustement! instead of GtkObject. This should
also be changed.

> * the files gda-value.h and gda-commandP.h in the gda-client directory
> seem to be orphans and could possibly be removed
>
yes, they seem so, but don't remove them yet. I'll have a look at them

> * if we make use of the popt library in gda-server-init, we must honour
> that in configure.in, because that library is not necessarily installed
>
yes, you're right
 
> * we could put some ..._LIBADD in the Makefile.am's to link our libraries
> to the correct dependency libs if dynamic libs are used.
>
I don't nderstand very well what you mean by this.
 
> * and finally we could fix all the compiler warnings that are still
> generated when building gda.
>
yes
 
> Of course I would volunteer to do those things, if you want me to.
> 
> What do you think?
> 
apart from the comments, it looks perfect. So, please tell me on which things
you want to work and I'll help you doing the other ones.

Thanks very much for a so concise analysis.

cheers