Re: [Evolution-hackers] New 'eclient' branch in eds

[Matthew Barnes <mbarnes redhat com>]

On Tue, 2011-04-19 at 07:58 +0200, Milan Crha wrote: 
> > * In EBookClient, drop the 'const' qualifier from EContact arguments.
> >   Trying to use 'const' with GObjects is misguided and pointless.  I've
> >   cursed at EIterator many times for this.
> 
> Yet another thing I wanted to address was a clear memory management
> recongnizable from function prototype. Thus if the function doesn't
> change object's/variable's content, then I made it 'const' to indicate
> it's used for reading only. Of course, inconsistencies all around the
> code in this makes it hard to do right, so the function type-casts it
> back to non-const pointer internally, because EContact API is not
> written in that way. (Though it's not only about EContact.)

Havoc Pennington kept having to answer this same type of thing in the
early days of GLib/GTK+ when people would ask why the API never uses
"const" in functions that take but don't modify a GObject and GLib data
structure.

http://mail.gnome.org/archives/gtk-devel-list/2001-May/msg00485.html

The take away there for me has always been "const" is useless for any
kind of struct that has a pointer member, which virtually all GObjects
do.  The API docs are a better place to document that the argument is
not modified.  Don't try to do it in the function prototype.


> > * Why do we have "get_capabilities" functions in EClient, ECalClient and
> >   EBookClient?  Are they different sets of capabilities?  And if getting
> >   capabilities involves a D-Bus call then doesn't it need to be async
> >   everywhere?
> 
> They are same capabilities. Same as ECal/EBook the EClient caches
> capabilities once it's asked for them, and reuses them, same as is
> responsible for its memory. The cached values are also used for
> capability checking. These are fetched on demand, and are always
> synchronous. For cases where this is unusable, and where the caller will
> take care of the memory, I added also get_capabilities functions to
> ECalClient/EBookClient, which has both sync and async versions.

Hmm, so you're saying I first have to fetch the capabilities via the
async calls in ECalClient and EBookClient, then the result is cached in
EClient?  I'd suggest renaming the EClient function then to something
like e_client_get_cached_capabilities().


There's a couple other simple things I forgot to mention yesterday.

* We should add some padding to all the public class structs so we can
  add new class methods in the future without breaking ABI.  Something
  like this:

        struct _ECalClientClass {
        
            ... methods and stuff ...
        
                gpointer reserved[16];
        };


* Also, GLib 2.26 gained its own timezone API: GTimezone.

  http://developer.gnome.org/glib/unstable/glib-GTimeZone.html

  It would be good to eventually try and correlate GTimezone and
  icaltimezone.  Other projects will be using GTimezone now since
  it's part of the GNOME platform libraries, and will likely expect
  to be able to use GTimezone with E-D-S libraries.

  I'd like to add least make room for this in the name space now by
  renaming all functions that expose libicaltimezone pointers from
  "timezone" to "icaltimezone".

  For example, for now we'll have:

  void e_cal_client_set_default_icaltimezone (ECalClient *client,
                                              const icaltimezone *tz);

  and eventually we can add:

  void e_cal_client_set_default_timezone (ECalClient *client,
                                          GTimezone *timezone);

  which converts the GTimezone to icaltimezone internally.