GNOME.org
 

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

�ic Bischoff a �it :

Le Jeudi 15 Juin 2006 17:40, Julien PUYDT a �it :

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. 

Well, then you reduced the question:
"How do I tell the driver which contacts I want?"
to:
"How do I tell the driver which identifiers I want?"

I don't find this impressive.


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.
It *must* be at the core of the system. We _do_ want to update the live data when it changes. 



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. 

What do "before" and "after" mean ?


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?
I certainly have issues with big books, and must find some satisfying way to handle them. But doing function calls to walk on a list where I could get the list and walk it myself directly, is definitely wrong. 


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".
Notice that the only place where you call the _get_contacts function is when you first populate the book. The rest of the time you don't want to know.
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.
Most operations are sequential when the book is static. When your contacts go online/offline/whatever, that certainly is random access, and I can tell you a first/current/next approach doesn't fly in such a situation. 


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);
		...
	}
Yes, of course. You put everything at the high level, something which I already stressed was very very bad.
Let's try an example. A text editor will just access files. It won't wonder if they are local or remote, it won't wonder about where the data really is on disk, need caching, etc. This is the job of the low-level system. The text editor doesn't have functions and tests to handle all of the various situations possible. High level is high, it doesn't take mud baths with the details, it doesn't handle them. It delegates them to the low level. 


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 ;-).
Partially wrong. The generic view can indeed get access to some specific features (like the menus I explained). But it certainly *must* not get them by handling *all* possible situations. 


	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).
Such style of coding is not classical, is bad and evil. I see abstractions layers everywhere to hide the details of the lower level, and you want to push those details high up the stack. 


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 ;-).
You push all the code to the high level, and never really abstract. That is very wrong. 


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.
If I add a new addressbook type with a Foo capability, your designs says I have to modify the base class and add: 

bool canDoFoo ();
void pleaseDoFooThisWay (...);
void pleaseDoFooThisOtherWay (...);
void etcFoo (...)
(with all the virtual pure syntactic crap needed)
My idea of right design is that if the new addressbook is the only one needing Foo, it will handle it himself (through a specific api that its view and controller will know and use). 


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).
I agree I have issues seeing how to do things *properly* with a very large addressbook. 


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.
It isn't much more generic. It is an awful and indigest mix of all the specific implementations.
And if the same capability exists in a second address book, the code is already written. 

Not really.


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 ;-).
Which are very useful and allow to abstract common work at a lower layer and not take care of it ourselves way up.
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. 

Your view isn't OO, and isn't C++.


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 ;-).
I see functions spread in maintainable files, with well-separated features and responsibility.
You see functions named "methods" stuffed into the same poor base class, which has to handle everything and the rest, and where the limits between everything is blurred. 

Where I say function and do:
gm_contact_do_foo (contact, arguments);
you say method and to:
contact->do_foo (arguments);
which I agree is shorter, but isn't the magical solution you think it is! :-) 


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.
You *do* have isReadOnly and canSubscribeContacts, which you will use to do the ifs I said where very wrong, and you will do so at the high level, which is precisely the place where details shouldn't be handled! 
It isn't a case of leaky abstraction, it is a case of no-abstraction!
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.
I generally end up with nice layers of functions doing almost nothing, with clean api between the layer, ensuring the code will be pretty maintainable.
See lib/gmconf/ for example. We have a nice api in gmconf.h, and an implementation in gmconf-glib.c, which is layered in the same file, with lowlevel functions doing some work, and higher level doing another. Not everything done at the same final level.
When I wrote that code, the goal was to make gconf (the gnome configuration management system) optional, so the port to win32 would be easier. I wrote a generic api to handle things, then two implementations *hidden* behind it.
Well, at least we both agree I don't handle the case of very large addressbooks very well :-/ 

Snark
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]