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

[Matthew Barnes <mbarnes redhat com>]

On Thu, 2011-04-28 at 11:56 +0200, Patrick Ohly wrote: 
> First of all, +1 for rethinking the API. I'd like to suggest that
> besides modernizing the API we also take this opportunity to move more
> of EBook/ECal into a common core.
> 
> Opening and listing databases don't have to be separate. Turn
> ECalClientSourceType into EClientSourceType and all of the _new,
> _new_from_uri, _new_system, _new_default, _get_sources functions can be
> moved into e-client.h.
> 
> The advantage would be that clients (like SyncEvolution) which work with
> both only need to implement the common operations once. Right now I have
> a lot of duplicate code in the address book and calendar backends.

That's not a bad idea, actually.

It would mean EClient has to know that ECalClient and EBookClient exist
in order to instantiate the right one for a given enum value.  Normally
in class design you want to avoid that kind of knowledge seeping into
lower layers, but with the class hierarchy being fixed to these three
classes (four if we add email), I think it's a good tradeoff to have a
more consistent API.

So it would be something like:

typedef enum {
    E_CLIENT_TYPE_ADDRESS_BOOK,
    E_CLIENT_TYPE_CALENDAR,
    E_CLIENT_TYPE_MEMO_LIST,
    E_CLIENT_TYPE_TASK_LIST
} EClientType;

I'd prefer *our* terminology in the enum over iCalendar terminology.  I
always have to stop and think "okay a memo list is called a VJOURNAL"
and so on.


> Matthew suggested to replace some of these with
> e_source_registry_get_default_calendar/address_book:
>         
>         e_cal_client_new_default()
>         
>               Instead do:
>         
>               source = e_source_registry_get_default_calendar (registry); 
>               client = e_cal_client_new (source, E_CAL_SOURCE_TYPE_EVENT, error);
> 
> The same logic would apply here: instead of having separate calls for
> calendar and address book, have one call with an enum.

Yeah, having one "get_default" function would be desirable.

The functions would have to be moved to e-client.h in order to use the
enum, but now that I think about it, it kinda makes sense to move them
to e-client.h regardless.  The default values only come into play when
you have to create a client connection to *something*.


> BTW, in this particular example, wouldn't
> e_source_registry_get_default_calendar() need such an enum anyway to
> distinguish between default events, tasks and memos?

I also wrote:

   e_source_registry_get_default_task_list()
   e_source_registry_get_default_memo_list()

http://mbarnes.fedorapeople.org/account-mgmt/docs/libedataserver/ESourceRegistry.html

So there's four "get_default" functions all together.  Hence why having
a single enum definition to cover them all would be nice.


> Talking of capabilities, I found their usefulness a bit limited because
> they a) cannot change during the life time of a client and b) they only
> report "exists/doesn't exit".

I'll leave it to Milan to address the capabilities stuff.  He knows more
about it than I.