KMessage/KMessageHandler: a core/ui seperation for displaying message.

[Michaël Larouche]

Le May 28, 2006 15:02, Matt Rogers a écrit :
> On Sunday 28 May 2006 13:16, Frans Englich wrote:
> > On Sunday 28 May 2006 17:06, Michaël Larouche wrote:
> > > I rencently wanted to make a notification framework for Kopete4 to
> > > replace all the KMessageBox in the code. Then Olivier Goffart
> > > remembered me of the thread "core/ui separation".
> >
> > Nice work. My comments:
> >
> > * It is heavy to make such a generic class as KMessageHandler a QObject
> > for the only purpose of memory management, imho(or is there any other
> > reason?). I would use a reference counting mechanism instead, since it
> > doesn't decide that all users should use QObject, and it is also a more
> > flexible memory management mechanism.
>
> Yes, use KSharedPtr or its equivalent instead. If reimplementers of the
> generic message handler need a QObject, then they can use multiple
> inheritance.

I used QObject because QPointer need to be a QObject derivate. But using 
KSharedPtr make sense too.

> > * I would like to put KMessage and KMessageHandler in separate files to
> > make things easier to navigate/less to include, etc.
>
> how is it easier?

I normally do this do, one file per class, but I found KMessageHandler too 
small to be put in a separate file.

> > * Make the values in MessageType OR'able(1, 2, 4, 8), such that they
> > later on can be used in code which needs to say "I accept these kinds of
> > messages", and so on. Add "typedef QFlags<MessageType> MessageTypes;"
>
> IIRC, values don't need to be OR'able in order to be used by QFlags. Just
> the proper Q_DECLARE_FLAGS( MessageTypes, MessageType ) macro needs to be
> used instead. AFAIK, "typedef QFlags<MessageType> MessageTypes;" is less
> correct.

I prefer Q_DECLARE_FLAGS myself.

KMessage doesn't offer many possibilies so I don't see why I should 
overengineer it to check for capability of the handler.

> > * I would put the private parts of KMessage in the cpp instead, since
> > it's public API. (Or no, it wouldn't work?) It would also be nice to get
> > the other private functions out of the public API. Since d-pointers
> > should be added anyway I guess they can be moved there(no?).
>
> what do you mean here? Where do you see the need for d-pointers? I don't
> think a d-pointer in necessary in this case.

Yes, I didn't put d-ptr because there is no need for it. I didn't want to 
manage a d-ptr just for 1 or 2 private members. And yes, you can put private 
members into a d-ptr

> > * Doxygen is automatically inherited from super classes, so it's not
> > necessary to @copydoc it(well, Adriaan, correct me on that if I'm wrong).
>
> better safe than sorry, no? :)

exactly, like we said in French "Vaut mieux prévenir que guérir" (Better 
prevent than cure)

> > * I would tone down some of the test code: "You revealed Apple's secret
> > and you are now sued by them", and "Loading of kde_pron". I would come up
> > with other texts.
>
> agreed.

I changed that to "You have turn to the dark side of the Force *imperial 
march*" and "Loading failed. Your user experience will be affected."

> > * To me it looks like calling KMessage::short(type, text) is identical to
> > calling KMessage::shortMessage(type, text). If they're not, why/when
> > would one want to pass an empty QString to short()? I also think it's
> > better to code information about a message in data, instead of the name
> > of a function call.
>
> where is there a KMessage::short function? I don't see it.

it is shortMessage(). First I thinked of amaroK status bar message that 
display short/small message for confirmation of a operation. I use the same 
system as amaroK in a personal project of mine.

statusMessage() will be a better name like Matt suggested.

> > * What reasoning concluded KMessage::Sorry is needed? Debug, Warning,
> > Error, Fatal are all "message types" that can easily be distinguished and
> > motivated(imho), and they aren't controversial. For example, it's what
> > DOM 3 Core and Qt happens to have. However, what is a message that is of
> > type Sorry? Why isn't a warning or fatal error of type Sorry? What is the
> > possible actions a message handler can do on Sorry? I think that a need
> > of "Sorry" originates from that this KMessage/KMessageHandler is
> > insufficient in some cases.
>
> This would be analogous to the current static functions in KMessageBox like
> sorry, warning, etc.
>
> You say it's insufficient, but you don't say how. How is it insufficient

Yes, it is analogous to the KMessageBox function. I started it simple. Then I 
posted my result of kde-core-devel to get input.

> > It could be an idea to abstract the message into its own class,
> > "KMessage", that instances of are sent to the message handlers. In this
> > way one doesn't have to distinguish between short/shortMessage, and users
> > can sub-class KMessage to make them carry exactly what they need.
> > KMessage would have the pure virtuals "MessageType type() const",
> > "QString message() const", and "QString title() const", perhaps. DOM 3
> > Core uses this design, obviously I find it sensible.[1]
>
> Why would we need this sort of functionality at this point in time? If it's
> needed later, we could surely add it. Let's not get carried away with
> trying to do handle everything all at once.
>
> > I don't really get how this is supposed to be used. The purpose of
> > message handlers is to abstract the code, to give flexibility. That a
> > global, static message handler is used counter acts that. Who is entitled
> > to change that message handler? Some library in kdelibs? A user
> > application? I don't know about all usage scenarios this is intended for
> > so it's a bit difficult to comment and propose alternative solutions. All
> > I can is that a global message handler is limiting, and that one wants to
> > move away from global things, not increase it.
>
> At this point, I can't say I understand it either. Especially with the
> examples provided.
> --
> Matt

-- 
Michaël Larouche
KDE developer working on Kopete, Kamefu, Solid...on dial-up :P
--------------------------------------
Website: http://www.tehbisnatch.org/
MSN/Email: michael.larouche at kdemail.net
IRC: irc.freenode.org/DarkShock on #kopete,#solid,#kamefu,#plasma
Jabber: darkshock at myjabber.net