[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kopete-devel
Subject:    Re: [Kopete-devel] [Long] next release
From:       Martijn Klingens <klingens () kde ! org>
Date:       2002-04-29 18:14:15
[Download RAW message or body]

On Monday 29 April 2002 18:05, Andres Krapf wrote:
> yup, but think of the plugins which do not use a QMap (aim/icq/jabber
> don't). when they receive a message, they currently have no way to do the
> mapping. their solution is the following: connect each KopeteContact to
> signal messageReceived, and each contact compares the sender's ID to it's
> own. the one which matches emits a messageReceived(foo,this). and this
> leads to less-than-obvious code to read. subobtimal too, but that's not a
> real problem (we all have fast machines right ? :-)

I think QMap is one of the best approaches for a clean design (or QList, or 
QWhatever, at least maintaining the list internally to the plugin), but I 
agree that it is quite a lot of work to do that. I know from personal 
experience, because it cost be a hell of a lot of time to convert all the 
ugly MSN code (which at best could be described as 'improved functions') to 
something remotely resembling a good object oriented design. And there is 
lots more to do before I would even consider saying 'hey, MSN has a clean 
design, use it as your reference protocol'. It's not and it won't be soon. 
Still, compare current MSN with the MSN from roughly two-three weeks ago and 
you see an enormous improvement.

> now i want to introdutce KMM in the AIM plugin. so i need to do my mapping
> right. if i go the QMap way, i'll have to track every place of
> (creation|modification|deletion) of contacts inside the plugin, to also
> update my QMap. that's what i mean by syncing.

If the design of the code is already clean and object oriented that would be 
easy. Otherwise it will be a true pain.

> if i use a UniqueId solution, i only have to change the creation of the
> contacts (new AIMContact()) to include the uniqueid. that's only about one
> or two occurrences, usually.

Well... read all the CVS logs for MSN I'd say. I converted the old code (which 
probably largely resembles ICQ/AIM) to what I have now gradually. There is a 
method msnId() internally which was used in a lot of places. I also had 
several additional helper methods that I gradually phased out when more of 
the API was capable of using MSNContact directly.
You could do the same, using something like

AIMContact *ac = dynamic_cast<AIMContact *>( contact );
if( !ac )
  kdDebug() << "Not an AIM contact!" << endl;
else
  // search through list for the contact with aimId() == ac->aimId();

Note that dynamic_cast is FORBIDDEN across dlopened code like plugins, you 
should use qt_cast there, but inside a plugin itself it's safe. Don't ever do 
it from libkopete to a plugin, though!

> i hadn't thought of that, but using UniqueId would allow you to do
> something like KopeteContact->getUniqueId(), dynamic cast this and then
> ->getProtocolId()

MSNProtocol::protocol(), anyone? ;-)

That is, if all that stuff is done in the plugin, where it belongs, not in 
libkopete.

And if you name accessor methods getXXX you might find yourself receiving a 
flame from me ;-) In KDE/Qt it's setFoo(), but the accessor is foo(). Please 
don't introduce an inconsistent naming with Qt/KDE's libs...

> anyways, i've looked at the QMap in msn... i had no idea this was so
> extensively used there. i understand your concen about changing things...
> how about the following then:

MSN didn't have a QMap two weeks or so ago. I added that because it felt most 
natural. And it worked out so extremely well that I'm now trying to push it 
into the other plugins as well. I know from personal experience that you can 
easily cut 30% of all list traversal code out, making the code less complex, 
more readable and easier maintainable.

> introduce a UniqueId that will perform as mentioned. that way:
>  aim/icq can be more easily ported to KMM.
>  jabber might use it to change the flow of the incoming messages to not go
> to all KopeteContacts. (more readable/logical)
>   the only change to msn would be create a dummy MSNUniqueId to include at
> creation time in the contacts. not too difficult...
>
> would that be ok ?

Well, again, instead I'd favour the temporary introduction of unique ids in 
the plugin itself and doing the processing there. That will also likelier 
force the developers to move the code to sane data structures on the long 
run, even though in the short term it's a pain to port. (I know, been there, 
done that myself.)

Martijn

_______________________________________________
Kopete-devel mailing list
Kopete-devel@mail.kde.org
http://mail.kde.org/mailman/listinfo/kopete-devel
[prev in list] [next in list] [prev in thread] [next in thread]