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

List:       kmail-devel
Subject:    Re: [PATCH] UI abstraction proposal (partially only)
From:       Andreas Gungl <Andreas.Gungl () osp-dd ! de>
Date:       2003-06-03 7:42:43
[Download RAW message or body]

Am Dienstag, 3. Juni 2003 09:31 schrieb Cornelius Schumacher:
> On Tuesday 03 June 2003 00:00, Andreas Gungl wrote:
> > attached you find a diff containing a first idea on how to separate
> > the UI code from the processing code. It covers only the address book
> > access in KMail.
> > I formed an abstract class for the UI callbacks, then I inherited a
> > class covering the current Gui interactions.
>
> While this in principle makes sense (we have similar classes in libkabc
> and ksync), I think it would be better to store the callback object in
> a member variable instead of passing it as parameter to all functions
> using it. That makes the API much clearer.

I've also thought about this. Unfortunatly the KMAddrBookExternal class has 
mainly static methods, that's why I passed the callback instance as 
parameter. In general, I would tend to use member variables too.

> > I would like to have some comments on the patch, especially on naming
> > conventions and related topics. E.g. the classes are named
> > *GuiCallback, IMO it's better readable than *UiCallback, although
> > text UIs are of course possible.
>
> I think "Ui" is better than "Gui" because it's more acurate. In addition
> I don't think that it's necessary to include "Callback" in the class
> name as it doesn't provide any information what the class is meant to
> do. My suggestion would be to name the class "AddressBookUi".

Thanks. Let's see if there will be more comments on this. But your 
suggestion is fine with me.

Andreas

_______________________________________________
KMail Developers mailing list
kmail@mail.kde.org
http://mail.kde.org/mailman/listinfo/kmail
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic