Details about the improvements in 2.1

Large changes in 2.1:

  • A lot of code changes (see also below)
  • kDebug(), kWarning(), kError(), kFatal() policy changes
  • Splitting the contact list into a nice contact list object with i.e. ->swapGroups(Group*,Group*), and a model like it is now
  • A completely new P2P stack
  • Lots of new eye candy
  • A connection checker to see if P2P will work correctly
  • A debug log gatherer in the LikeBack client frontend, and integration with the crash handler

Improving Code Design

KMess has always had the current (as of 2.0) code structure:

  • network classes attached to GUI and data classes
  • current account copying account data over stored accounts
  • everything uses currentaccount, but to actually do stuff, we need to send signals back to class KMess or somewhere near it (see some signals in ContactFrame)

This design doesn't allow anything more than we have now, and is already showing all of its limits.

After some thoughts exchange here is a first very rough draft of how we can re-design the KMess code to allow for these important points:

  1. Allow to separate the GUI from the network code
  2. Add flexibility to the design (i.e. allow using the code in previously unexplored ways)
  3. Reduce its complexity and make it more robust and logical
  4. Allow to merge and separate single components as needed

This is the proposal. -- the class names are placeholders --

CurrentAccount is renamed to "KMessSession". It is no more a subclass of Account but a completely separate entity. Its purpose is to be the only interaction point between the Live servers (backend) and the application components (frontend/GUI). It only holds data retrieved from the server (contactlist, verified account status, webmail support status, etc), which we call 'volatile data'.

** When the library gets created, KMessSession will be part of the library and will be the only part exposed to KMess - Account will be split from it (no Account->CurrentAccount inheritance anymore) and kept by the KMess GUI **

Account class instances are used directly, without any copying. The logged in Account instance will be available as a global shared Account instance, and the current KMess session will be available as a global shared KMessSession instance. This is not a lot better than our current CurrentAccount singleton design, but all of KMess' GUI is built around it and there's no need for us to take the pain to change it now. KMess will stay single-user as long as this design stays as it is, and we're not (yet) intending to change it anytime soon.

All current calls to CurrentAccount::instance() and all currentAccount_ members will need to go. Inside the network library, they will be changed to KMessSession calls or parameters. For example, when MsnNotificationConnection logs in it currently asks Account for its initialStatus, but then that will be changed to an argument to KMessSession::logIn(), Status initialStatus. When a GUI class needs to access a setting, it gets "extern Account globalAccount;" at the top, and then it can use globalAccount->getSetting().

When the user tells KMess to connect:

  1. the Account instance for the chosen account is picked
  2. a new KMessSession is instanced and set in the global variables
  3. KMess feeds the KMessSession the basic data from Account needed to connect (username password and the like)
  4. KMessSession connects, and the correct Account is set in the global variables
  5. the correct globalSession and globalAccount are set when any activities start.

Edit: The above is now done! Except that KMessSession is instanced at start and never destroyed, all (or at least, most) text above reflects the current situation as of r5108, r5110, r5111 r5112 and r5121. A lot is still TODO, see also the next section.

Consequences

First and most important: GUI and network data get finally separated. That's step #1 for the library split. Second, the data, the GUI and the network code get isolated. Third, the code will be cleaner and easier to learn and use.

Some details Diederik and me (Valerio) thought about: need to be changed to the current proposal of having two global variables

  • If the SessionContainer is a base class of all classes which need access to session info (network or account data), we can use the parent-child relationships in Qt to pass along the data: ie if (say) KMessView and ChatWindow both inherit from SessionContainer class, we can make a KMessView with the session data, then instance a ChatWindow with KMessView as its parent: the SessionContainer constructor will get the parent SessionContainer's data pointers and use them.
  • Since all components know which session they're part of, we can connect more than one account at once. not yet
  • If the main KMess GUI code remains separated from the SessionContainer data (acting only as a shell for SessionContainers) we could even have a Kopete-like joint contact list - the KMess GUI code can retrieve the contactlists from all connected accounts, join them, and display them at once.

Sample implementation

See the history for this wiki page to see the sample implementation which uses SessionContainer. The current proposal doesn't use that class, so it has been removed for the sake of relevance and being clear.

Implementation changes

Everywhere in the GUI:

  • Get rid of the bad habit of passing internally "QString handle" as a parameter to calls, and use Contact* everywhere possible

Everywhere in the network stack:

  • Don't use Contact* and other GUI classes -- Disagree. The concept of a "Contact" and "Group" is perfectly valid in the network stack, at the very least for grouping GUIDs and names together. I propose creating Contact/Group classes for the network stack that only contain information relevant to the protocol, then creating derived classes outside the stack for use in the GUI (KMessContact / KMessGroup for example). Then use Contact * and Group * in the stack's API where you'd use &handle and &groupid. Advantages: a clean, simple and intuitive interface to the outside world. - Adam

I definitely agree :) --Valerio

Thinking about this more, a derived class won't work (since I guess the network stack would create its Contact/Group objects internally then pass them out). But KMessContact/KMessGroup classes that contained a pointer to their associated network-stack Contact/Group object would work. We did want to stop using QString &handle everywhere :) -- Adam

KMessView:

  • Separate the Contact List tree widget into a separate widget done as of r5124 and r5126

Model Changes - Shifting View Logic

The model contains logic for ordering and swapping contacts/groups for the view. This is view-level code. To that end I propose moving the sorting/swapping view code into ContactListModelFilter (where it belongs) and ensuring that ContactList only contains methods for working with the model itself - add/remove/modify contacts and groups, get contact by handle, get group by id, etc. (done)

Also, we should make the sorting of contacts a little more intelligent. Sort by status first, but then fully alphabetically also (done)

Moving most of this code should slim down the model quite substantially and make it easier to maintain and debug.

Also, the contact list save code needs to come out too. Where to put it, I'm not quite sure yet. (done - it went into the KMess class as (load|save)ContactList)

Edit: mostly done as of r5222.

Other changes

UI files:

  • Check the KDE HIG, verify compliance, correct issues.

Settings pages UI files:

  • Remove the : at the end of all group names. Check that no punctuation is present at the end of labels, except for text input labels; they need to end with that ":".