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

[Patrick Ohly <patrick ohly gmx de>]

On Do, 2011-04-28 at 08:17 -0400, Matthew Barnes wrote:
> 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.

I agree, it breaks layering, but overall I'd prefer such a unified 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.

Please, let's avoid "E_CLIENT_TYPE_CALENDAR". For people less familiar
with Evolution terminology it is not clear whether this includes tasks.
In Evolution, it doesn't, but in other contexts it does. For example,
Nokia phones store events and tasks in the same "Calendar". In
SyncEvolution I followed the Evolution terminology and "calendar" has
caused a lot of confusion.

It's not even obvious inside Evolution, because libe*cal*[endar] also
does include tasks and memos.

What about E_CLIENT_TYPE_EVENTS instead?

> > 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.

Here we have the first misunderstanding because of "calendar" - it
wasn't obvious to me that this was exclusively for events ;-}

-- 
Bye, Patrick Ohly
--  
Patrick Ohly gmx de
http://www.estamos.de/