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

List:       kopete-devel
Subject:    Re: [Kopete-devel] patch, and other stuff
From:       Martijn Klingens <klingens () kde ! org>
Date:       2002-05-05 15:29:11
[Download RAW message or body]

On Sunday 05 May 2002 16:28, Andres Krapf wrote:
> they were convenience methods... i didn't see anything wrong about easing
> the life of the users of this class. honestly, i don't really care about
> them, if you don't like them i'm ok with getting rid of them. i'd
> appreciate having a reason though :-)
> anyways, i'll leave this to your judgement.

Well... I *thought* it was possible to do the following:

QPtrList<QObject> list;
QObject *obj = new QOject;
list = obj;

With the last line initializing the list to a list of exactly one element. 
Daniel's remark also seems to indicate that's possible. Qt in that case does 
the conversion from pointer to list, with no explicit method needed. Hence my 
remark.

However, the Qt API docs seem to indicate this method is not there at all, in 
which case I think it's indeed useful to have these methods.

Summarized: the convenience for the caller is definitely needed, but please 
find out first if you need an explicit method for that and only add the 
methods if they are really needed :-)

> > > > > +       void done() { emit done(*this); }; // call when a message
> > > > > is done
> > > >
> > > > This one sounds *very* redundant to me. Please don't...
>
> it's not: if we choose to emit the signal from the message (right now i
> have nothing better - discussed below) then the emit line has to be
> somewhere in the KopeteMessage code. that's how qt works, i believe.

Well, what's wrong with explicitly doing emit sent() instead of wrapping it in 
a one-liner method? That's what seemed redundant to me...

> > signal KopeteProtocol::messageSent( const KopeteMessage *msg );
> > instead? IIRC there was an argument against that as well. Andres?
>
> yes, i would like this most, but each KMM would then have to connect to
> protocol::messageSent. that means, each chat session would receive
> notifications about each and every sent message, not only the ones it cares
> about.
> but you're right, i'm not fully satisfied with the current state of things
> either. although i believe it's better than nothing :-)

Hmm... 

signal KopeteProtocol::messageSent( const KopeteMessage *msg,
    const KopeteMessageManager *mgr );

maybe? The slot then starts with

if( mgr != this )
     return;

Maybe a bit radical, but it might be the solution. It shouldn't be too hard 
for the protocol to make it find out the proper KMM*...

> > I think the message should be put on hold until the actual sent() signal
> > is emitted (from whatever class we put the signal in ;-). In the slot
> > that handles that signal we may indeed mark the message as read. Hmm,
> > what about a property bool read(), void setRead( bool b ); in
> > KopeteMessage? Just a (very) wild thought...
>
> yeah, my first attempt at this was the setRead() method, without signals.
> this works nicely, and gets us rid of all the nasty qobject stuff in
> KopeteMessage. but then i realized that sending messages is an asynchronous
> process. which means, as you say, that you still need a signal to know when
> to act. and as i stated earlier, putting it in the protocol is no good.
> that's why i thought of the message.

But after the message is actually sent the KMM can still call setRead( true ), 
no? It doesn't have to be done before the data is sent, but that doesn't mean 
setRead() is a useless method either. Or am I missing a point?

> ok, i do believe you, but how is passing a reference different from passing
> a pointer ? and why does one lead to crashes and not the other ?

See below...

> ok i see... the problem therefore is not whether i use a reference or a
> pointer, but rather that i should allocate with new. am i correct ?

Yes. Once you use new you already have a pointer anyway, though, after which 
it's not generally considered good practice to keep on using references. 
There are uses for them, but usually if you're using pointers, keep on using 
pointers and don't mix them with references. Which is clearly the case here.

> another question is, who should delete it ? the most natural thing is to do
> so in KMM upon reception of the sent signal, but that would stop others
> from using that signal (since QT doesn't guarantee the order in which slots
> are called). therefore, i'll do it in the message itself, right after
> emitting the signal. point 4/

Depends on who can actually use the signal in the first place. AFAICS only 
KMM, KopeteMessage and the protocol actually have access to the data to 
connect to the signal at all. A message won't connect to its own signals, and 
the protocol emits the signal (be it directly or through the message - the 
protocol knows about the status change first either way through its very 
nature). That leaves the KMM, so essentially the signal is by definition 
mapped to a single slot. Which also means that we don't have to worry about 
multiple-connected signals here. A warning in the .h file that defines the 
signal not to connect to it yourself might still be in order though, but I 
don't think it's much to worry about here.

> as soon as it's ready, i'll send a new patch with 2/ 3/ 4/ included.
> i'll leave 1/ to your judgment (aka, feel free to apply the patch and get
> rid of those  convenience ctors if you like)

Looking forward to it :-)

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]