Re: [GnomeMeeting-devel-list] Ideas and considerations for the addressbook code

[Éric Bischoff <ebischoff nerim net>]

Le Jeudi 15 Juin 2006 17:40, Julien PUYDT a écrit :
> Saying "I get only the 10 I need" is nice, but how do you tell the
> driver which they are ?

There are several ways to acheive that. It could be contact numbers. Or it 
could be some data kept internally inside the driver. It could be a void *. 
Etc.

> > While this sounds cool, I am not sure that this will be possible in some
> > circumstances. The underlying address book (evolution, KDE address book,
> > LDAP server, Zeroconf, ...) might not have the mechanisms to send such a
> > signal.
>
> No problem : then they don't.

OK. So that should not be a requirement, and certainly not a mechanism at the 
core of the system. Just a nice plus.

> > Why transfer 200 contacts when the view knows that it needs only 10?
>
> The view may show only ten at the same time on the screen, but the mouse
> wheel makes it so easy to go through all of the 200, that it wouldn't be
> wise to have them ready.

Yes. You could have a few spare contacts before and after the user's view. 
This algorithm is rather common.

> If there are several thousands, that's more annoying.

Okay. Let's simulate your algorithm, or what I have understood of it.

The user views a search selection of contacts named "Tom". His/her window 
should contain contacts Tom Smith (#1032), Tom Jones (#2050) and Tom Doe 
(#3172). The high level software asks the driver "get me all contacts". After 
a relatively long time, the driver is wise and stops at contact 200. It 
returns a list with contacts 1, 2, 3, etc, up to 200. The high level software 
searches for Tom's in the list and finds none, since the Tom's have numbers 
1032, 2050 and 3172. The returned list is useless, and the window remains 
blank.

I know that it cannot be _that_ bad and that I certainly misunderstood 
something. But what?

> > The views know that contacts 301 to 309 are displayed. It's the view that
> > knows where to cut. In fact, there's nothing to cut, since all the view
> > does is ask the driver for the contects it needs.
>
> How do you number the contacts in a sane way?

These numbers are here for the demonstration. I agree that numbers probably 
are not so well adapted to real address books.

You could use "markers". I think that all address books use contact UIDs. A 
given UID could be used to mark the place where the address book stopped 
"last time".

For most usage you do not even need anything. Most operation is sequential. 
Take for example searching. Usually you browse all records and only keep in 
the result list the ones that match.

> I would put the following :
> 1) a title (generally the name)
> 2) a description (for example a location, a status message, etc)
> 3) an icon

Yes, we are discussing principles. Details can be arranged later.

> > The search result could handle much more detailed information.
>
> Of course it cannot. Your search result is a book which needs to be able
> to show avahi contacts, sip contacts, call history contacts, etc... so
> it can only show the contacts as generic.

Why would you be unable to display detailed information about objects which 
you only know as generic?

	while (genericContact = nextContact())
	{
		if (genericContact->book()->canSubscribeContacts())
			proposeSubscription(genericContact);
		...
	}

> The view which is specific to an addressbook, of course can show the
> data from the specific contact associated to it. But the generic view
> can only show generic information.

Wrong ;-).

Not shown in the code sample above, but that's also the whole point of virtual 
methods.

> The menu is supposed to contain actions which are specific to the given
> contact, book or view. Hence it must be created by code which is
> specific to that contact, book or view. It cannot be done by the generic
> code.

;-)

> > 	class addressBook
> > 	{
> > 		virtual bool isReadOnly() = 0;
> > 		virtual bool canSubscribeContacts() = 0;
> > 		...
> > 	};
>
> No, that is wrong and I already explained why. Putting everything in the
> base class is bad.

Do you realize that the above is not an implementation? It's just an API 
definition. Those are pure virtual methods. Everything is implemented in the 
derived subclasses.

Such style of coding is very classical, good, C++ (or java).

> > In your example, a signal is not needed.
> >
> > Knowing that there is no VoIP number in the current contact is enough.
>
> That is wrong. The same contact could update itself and gain it. Or
> update itself and lose it.

I should have added "at a given time".

> Which means :
> 1) *all* of the addressbooks would by default have *all* the possible
> apis for such capability by default, with a isAbleToDoFoo function for
> each Foo capability ;

No. Defining pure virtual functions in the base class does not mean that you 
implement a function in the derived class. If it can't do something, and lets 
the engine know it, then it can't ;-).

> 2) if an addressbook needs a new capability, we'll have to modify the
> base addressbook to add it in for it ; it won't just be a matter for the
> new code to add only what is necessary for itself in its own files.

Wrong again, C++ comes with programming techniques that permit to locate the 
real implementations in the derived classes.

> > It definitly deserves congratulations. You forgot to mention one of your
> > nice ideas, that it that it should be possible to search in several
> > address books at a time.
>
> Well, that is the main side-effect of having a base class for contacts.

;-)

> I think you are focused on an addressbook as something both very big and
> very static, which explains many of our divergences.

I think that we are both wrong here ;-). An address book could be big (which 
you underestimated), and it could be moving (which I underestimated).

> Yes, that's it. The high-level code doesn't know what an AvahiContact or
> EDSContact is ; it only knows avbout GmContact. That means it cannot do
> fancy things with them, and has to call functions on them to do anything
>   interesting.

I have a view that is not so far away. The book just knows contacts. Virtual 
methods and subclassing both the book and the contacts does the rest.

> Your high level code is very complex, and has to be modified heavily
> anytime we add a new type of addressbook with new capabilities.

Yes, but much more generic.

And if the same capability exists in a second address book, the code is 
already written.

> Of course, as-is that would mean a lot of code duplication. That's where
> we can use a toolbox ; of which you had an example in the first round of
> sample code I sent in private.

Ah, you too you can see the duplication problem with your method ;-).

A tool box... Sounds like a C library ;-).

I think that we basically foresee the same problems, but you want to do them 
in the procedural, C way, and I would like them to be done in the 
object-oriented, C++ way.

> To make it explicit for the -devel- readers, this sample code had a
> function:
> void shared_action_rename (GObject *obj);
> so any object which could be renamed, had only to:
> - have a "name" property ;
> - provide an action which trigerred that function on itself.
> the function handled showing a nice dialog to rename the function.

Yup. You see functions where I see methods ;-).

> The idea is that of course quite a few objects will have common
> capabilities ; but instead of putting them all in the high level code
> and clutter it with functions and conditional,

Yes, that would be the logical consequence if we were in C. We are not. We 
have virtual methods.

> I want to provide the 
> bricks which the low-level code will use to build a specific and
> well-fit system.

You will end up with an engine doing about nothing, and all the work to be 
reimplemented again and again in the drivers. Plus painfully shared code in 
the "toolbox" to avoid almost identical code where it can be avoided. That's 
typical procedural approach.


-- 
Concombre : "Alors, qu'est-ce que tu aimes dans la vie ?"
Chourave :  "J'aime médire."
(Le concombre masqué, Mandriva)