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

List:       kopete-devel
Subject:    Re: [Kopete-devel] more design discussions (an also,KMMF bugs)
From:       Martijn Klingens <klingens () kde ! org>
Date:       2002-05-10 23:58:03
[Download RAW message or body]

(Sorry for the late reply, but I have been reinstalling my server the past two 
days in the time that I was behind my computer at all.)

On Wednesday 08 May 2002 15:37, Andres Krapf wrote:
>  * first on, the bad thing about the way we use KopeteContacts (aka
> comparing pointers) is that nowhere is there a guarantee that the plugins
> won't create more than one per contact. so we either have to
>     1/ enforce the KopeteContacts uniqueness, somehow.
> or 2/ not rely on pointer comparisons, and overload ==. <- this might lead
> us to bad design with dynamic_casts (or qcasts), but i'm not completely
> sure.

Option 2 won't necessarily lead to bad design, but I am *strongly* in favour 
of 1. The MSN plugin should now be safe in that respect, I think I'm not 
using temporary MSNContacts anymore, and if I am I can adjust the code 
because I know my data structures support it. I cannot talk for the other 
plugins though. I think we should aim for this in the longer term and for now 
just _assume_ this is the case, fixing issues where they might arise 
(valgrind, anyone? ;-).

>  * next, for protocols supporting multi user chat, when a protocol needs to
> send a message with a KopeteContactList toKC, the plugin needs to map this
> KopeteContactList with whatever resource it holds that represents the chat
> session. this defeats the purpose of the KMM somehow. (i hope i'm clear
> enough here). here's where a chat session concept would be nice (as opposed
> to a mere list of pointers). the plugins would then just need to map the
> chat session to the underlying resource.

Well, KMM *is* the chat session... I think the contact list attribute can be 
moved out of the KopeteMessage class once all plugins use KMM. For now there 
might be code using KopeteMessage directly, so this cannot be done yet. Maybe 
in the branch? Not sure.

>  * in order to allow protocol personalisation of the chatwindow, having an
> optionsWidget() is no good. first because it's not extensible enough (not
> enough control is given to the plugin).

I discussed that tuesday night with Nick Betcher and Ian Reinhard Geiser. I 
hope something fruitful will arise from that discussion as I don't have time 
to code my share of the work the upcoming days.

> next, because the framework will
> not be able to check those values, so the protocol class will have to
> interact with the chatwindow directly (as opposed as going through the KMM
> and KopeteMessages).

The protocol should not even know about the chatwindow's existence, actually. 
The chat window is an implementation detail and should not be exposed in the 
design's API. KMM is the only interface to the chat window, direct access 
should be *strictly* forbidden. That's easy enough by moving the code out of 
libkopete into the src/ dir, so it's no longer public API...

> the sendThroughServer status, for example, could be
> nicely encapsulated inside the ICQMessage subclass for icq.

Subclassing KopeteMessage?!? Are you kidding? I'm not even happy with 
KopeteMessage inheriting QObject, and I definitely don't want to go the 
polymorphism way with it. I agree that it might offer some nice options, but 
I think it makes the API too complex. KopeteMessage is supposed to be small, 
lightweight and simple to use. By making it a polymorphic class, inheriting 
QObject and adding virtual methods it will be none of that.

> the best way i see to keep using the nice KMM functionality while allowing
> full customisation is either to allow subclassing of KMMs (which i think is
> a good thing).

Again, KMM should be a single entry point that just encapsulates messaging. If 
there is anything in KMM that could even remotely require plugin-specific 
changes then I think our current design is severely broken.

If you think about the classic 3-tier architecture, then the plugins are the 
3rd backend tier, KMM is the 2nd middleware tier and the chatwindow is the 
1st GUI tier. That also automatically defines explicitly that chatwindow and 
plugin can never possibly access eachother without interfacing through KMM.

This image is slightly blurred, because it assumes plugin == protocol. In 
reality the plugin also implements some methods to manage a GUI, but those 
should be kept at the bare minimum. Also, those methods should ideally be 
considered 1st tier and hence communicate with KMM and not with the protocol 
directly, if possible.

> if we go that way, there's the probem of the factory. we could use a
> factory method provided by the plugin, but then we'd have to dynamic cast
> like here:
>
> (inside, say, AIMProtocol class)
> /* this calls the KMMF::create which in turns calls some method provided by
> the plugin which creates a AIM specific AIMProtocol*)
> */
> KMM* kmm = sessionFactory()->create(blah);
>
> /* we have to dynamic cast */
> AIMProtocol* aimkmm = dynamic_cast<AIMProtocol*> kmm;

??? Are you casting a KMM to a KopeteProtocol subclass???

> a better solution might be to use a factory template. like this:
>
> template <class KMMT> class KMMFactory {
> 	KMMT create (blah) {
> 		if exists blah inside KMM list
> 			return KMM list item;
> 		else
> 			return new KMMT(blah);
> 	}
> }
>
> so that we reuse the code, but each protocol gets its own factory returning
> its own KMM (AIMKMM, ICQKMM .....) and we don't have to dynamic_cast.
>
> we could still provide the generic KMM for plugins which don't require any
> customization (or aren't there yet).
>
> comments ?

Yes: what the heck do you need this for? It is extremely complex, messy and 
sounds seriously overengineered to me. Is there a *need* for this or are you 
thinking about things like 'well, we _might_ want to do blah in the very 
distant future and then we need this' ? If so I'd rather leave things as they 
are now, fixing what needs fixing *now*. I want all plugins to use KMM ASAP 
and stabilize to get Kopete 0.4 out of the door based on it. After that we're 
a long way to shared code and then it might be a nice time to talk about the 
API again. We're not heading for BC yet, so please take small incremental 
steps. After all, open source is 'release early, release often', and this way 
the gap between Kopete releases will be *way* too big. If you need this to 
stabilize or finalize the current code then please explain and we might 
incorporate it. In all other cases: please wait at least until KMM is back 
into HEAD and a Kopete 0.4 is out. After that, start the discussion again.

> cheers (and sorry for the length),

Same here...

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]