Context Navigation

Introduction

When writing KMess code or patches, try to follow our coding standard as close as possible. Not only will it look ugly otherwise, but it's also harder to maintain code which follows different writing styles.

KDevelop's editor can be configured to follow the style automatically. Most of the coding style is based on these guidelines.

Some other parts of the code may differ a bit from these guides. Consider to correct those in a separate commit; don't clutter your SVN diff with coding style changes.

Common rules

The most basic, but most important things to get right:

Indent with 2 spaces.
Convert all tabs to spaces.
Put spaces inside parenthesis, not outside.
Put spaces around operators.
Leave 3 empty lines between methods.
Put all curly brackets at new lines:

  if( object != 0 )
  {
    ...
  }
  else
  {
    ...
  }

Always include brackets: (the only exception is the KMESS_NULL debugging check below)

  if( ! isConnected() )
  {
    ...
    return;
  }

Variable naming

When applicable, put the common name last. For example: itemCount, rootNode, bodyNode, okButton, fileMenu, contactWidget, fileDialog, enableNudgeCheckbox, startUrl, startPoint and errorLine.

It reads pleasantly, and avoids bad names like noResults. Such name makes it hard to decypher whether it's a boolean state or integer count.

Avoid abbreviations. They're hard to remember, so you constantly have to look things up. Standard abbreviations like HTTP, HTML, URL or GUID should not be expanded off course!

Try to use an is prefix for booleans. It solves a common problem of choosing bad boolean names like status or flag. Things like isStatus or isFlag simply doesn't fit, so you're forced to choose more meaningful names.

Instance variables

Instance fields are suffixed with an underscore.

  void Contact::setName( const QString &name )
  {
    name_ = name;
  }

Method naming

A "getter" method - a method which only returns a value - starts with "get". It's declared as const:

  void getName() const;
  void getStatus() const;

A "setter" method - a method which only changes an instance's value - starts with "set":

  void setName( const QString &name );
  void setStatus( const QString &status );

A boolean can start with "is" or "has":

  bool isOnline() const;
  bool isVisible() const;
  bool hasResults() const;

In method names and variables, abbreviations and acronyms should not be entirely uppercase. It conflicts with the camel case naming.

  const QString & getHtmlSource() const   // NOT: getHTMLSource()
  const QString & getHttpUrl() const      // NOT: getHTTPURL()

Method ordering

Many programmers have the habit to group methods together "logically". After some changes, it's no longer obvious how methods are grouped. Things easily become a mess and hard to follow.

Instead, order methods alphabetically. This way, it becomes easy to find methods in a large file.

Also keep 3 empty lines between methods.

Pointers and references

The * and & symbols are connected to the variable name. Also avoid multiple variable declarations at one line, because the pointer-symbol only applies to the first variable.

  QString &name;
  Contact *contact;

Alignment

Try to align operators whenever it enhances readability.

  name  = ...;
  email = ...;

  x = ( width  * size );
  y = ( height * size );

Enumerations

Recently, we started to write enumerations in the following way:

  enum P2PDataType
  {
    P2P_TYPE_NEGOTIATION = 0  ///< Default value, the session is negotiating.
  , P2P_TYPE_PICTURE     = 1  ///< Packet contains MsnObject data.
  , P2P_TYPE_FILE        = 2  ///< Packet contains file data.
  , P2P_TYPE_INK         = 3  ///< Packet contains an Ink message.
  };

By putting the comma before the first word it's much easier to add new items, and diff output remain clean.

The same also applies to constructors:

Contact::Contact( const QString &handle, const QString &friendlyName, int lists )
: ContactBase( handle, friendlyName ),
, allowed_( false )
, blocked_( false )
, friend_( false )
{
  ...
}

And long message texts:

  i18n( "You're invited by %1 to play the game %2"
      , contactName
      , game
      );

Complex expressions

Relatively simple expressions are written at one line:

  if( isRemoved || isBlocked )
  {
    ...
  }

For large expressions, write operators at the start of the next line:

  if( something to test here
  ||  something else )
  {
    ...
  }

If the expression becomes really long, consider storing the results in intermediate, self-describing, boolean variable.

  bool isFinished      = ( elementNo < 0 || elementNo > maxElement );
  bool isRepeatedEntry = ( elementNo == lastElement );
  if( isFinished || isRepeatedEntry )
  {
    ..
  }

Debugging

When testing for null pointers, use:

  if( variable != 0 )
  {
    ...
  }

There are situations where a variable never should be null. To avoid crashes, this convenient way allows you to add a precondition to the method:

  if( KMESS_NULL(variable) ) return;

When the variable is null, the KMESS_NULL macro outputs the name of the variable, method, filename and line number to the console. By having such convenient way, the checks are easily added and severe crashes are prevented. Off course, don't overuse this statement for situations where it's not needed. The KMESS_NULL expression is also the only exception for one-line if-statements.

Use kdDebug() to output the processing information. Put those statements between #ifdef statements, so they are only compiled with --enable-debug-output. The defines are declared in kmessdebug.h.

#ifdef KMESSDEBUG_CONTACT_GENERAL
  kDebug() << "changed status to " << status_ << endl;
#endif

Use kdWarning() for serious errors, and include the technical details as well.

  kWarning() << "Received HTTP status line: " << responseCode_;

  kWarning() << "PUnable to open file: " << fileName_ << "!";

  kWarning() << "Content-Type '" << contentType << "' not reconized, message ignored."'

  kWarning() << "Unknown argument encountered: '" << command[i] << "'";

  kWarning() << "Could not parse personal status message (invalid XML, contact=" << command[1] << ")!";

  kWarning() << "Unable to handle ACK message, original message not found "
                "(flags=0x" << QString::number(p2pMessage.getFlags(), 16) <<
                " size=0"
                " state=" << waitingState_ <<
                " contact=" << getContactHandle() << ").";

Normal errors are displayed in a KMessageBox off course. Technical details like protocol warnings are not displayed in message boxes. People will notice their picture wasn't transfered, or adding a contact failed. It's annoying - and not helpful at all - to display a message box each time this happens.

API documentation

We've just started to include API documentation, so most classes don't have it yet. You'll find some examples in the network/applications classes.

A class declaration example:

/**
 * @brief Data class for contact information.
 *
 * Each contact of the ContactList is represented by a Contact object.
 * It stores the data received from the MSN Messenger servers,
 * and maintains the state of the contact.
 *
 * @author Mike K. Bennett
 * @ingroup Contact
 */
class Contact : public ContactBase
{

This example is shortened. Note how the real version describes:

it's purpose
it's role in an mechanism
it's relation to other classes
the main things you need to know as well

Method documentation is put in the .cpp files as much as possible. This keeps the documentation tightly coupled with the actual implementation. API documentation only appears in the header file where needed: for signals, pure virtual methods, enumerations and instance fields.

An example of a public getter:

/**
 * @brief Return the identifier of the SLP session.
 * @return The Call-ID value from the SLP INVITE message.
 */
QString P2PApplication::getCallID() const
{
  ...
}

An example of a getter which plays a part in a bigger role:

/**
 * @brief Whether the transfer of file data was paused.
 *
 * Applications can be paused when a switchboard connection is currently busy
 * sending all pending messages. By pausing the application, KMess avoids
 * filling up the message queue further.
 *
 * @returns Returns true when the application is paused.
 */
bool Application::isTransferPaused() const
{
  ...
}

An example of an internal method. The method only parses some parts of an incoming message, to dispatch it to the actual handler. In this situation, the most descriptive documentation was it's role in a bigger part.

/**
 * @brief Called when a SLP status message was received.
 *
 * This method extracts the status code,
 * depending on the status code it proceeds with a different action:
 * - The <tt>200 OK</tt> message invokes gotSlpOk().
 * - The <tt>404 Not Found</tt> message invokes contactAborted().
 * - The <tt>500 Internal Error</tt> message invokes contactAborted().
 * - The <tt>603 Decline</tt> message invokes contactRejected().
 * - Any other message is rejected with a P2P error.
 *
 * @param  slpMessage  The parsed SLP message.
 * @param  preamble    The status line sent before the MIME fields,
 *                     which contains the error code (much like HTTP status codes).
 */
void P2PApplication::gotSlpStatus( const MimeMessage &slpMessage, const QString &preamble )
{
  ...
}

Translations

In KDE applications, translations are handled by wrapping a string in an i18n() statement. If you need to include other variables, provide them in the following way:

  i18n( "You received a message from: %1" ).arg( friendlyName );

Would the string be combined with the + operator, translators face so-called "word puzzles". There is no overview of the complete string. Some languages may require a completely different word order as well, making it impossible to translate that string.

Error handling

Abort directly on errors, this avoids deep nesting.

  if( ! isConnected() )
  {
    // ... report error
    return;
  }

The rationale behind this is illustrated with the following examples. The first example displays the recommended way. You'll notice the actual operation is done at the end of the method. All checks are done before that, and abort directly. It makes the code path is easy to follow.

  // Parse the data as XML
  QDomDocument xml;
  QString      xmlError;
  if( ! xml.setContent( responseBody, true, &xmlError ) )
  {
    kWarning() << "Unable to parse XML (error='" << xmlError << ")";
    return;
  }

  // Find the root node
  QDomElement resultNode = getNode(xml, "/test/example");
  if( resultNode.isNull() )
  {
    kWarning() << "Unable to parse XML: result node is null (root=" << xml.nodeName() << ").";
    return;
  }

  // Open a file to write results to
  QFile file( fileName );
  if( ! file.open( IO_WriteOnly ) )
  {
    kWarning() << "Unable to open output file!";
    return;
  }

  // ... do the actual work
  // ...

Compare this with the next example. Here you'll notice what happens with if-else based error handling. The code path is harder to follow because the error check and error handling are separated. The last error message is related to the first if-statement. The actual operation is somewhere in the middle between all checks and error handling. When a new error check is added, the SVN diffs will be cluttered by indention changes.

  // Parse the data as XML
  QDomDocument xml;
  QString      xmlError;
  if( xml.setContent( responseBody, true, &xmlError ) )
  {
    // Find the root node
    QDomElement resultNode = getNode(xml, "/test/example");
    if( ! resultNode.isNull() )
    {
      // Open a file to write results to
      QFile file( fileName );
      if( file.open( IO_WriteOnly ) )
      {
        // ... do the actual work
        // ...
      }
      else
      {
        kWarning() << "Unable to open output file!" << endl;
      }
    }
    else
    {
      kWarning() << "Unable to parse XML: result node is null (root=" << xml.nodeName() << ")." << endl;
    }
  }
  else
  {
    kWarning() << "Unable to parse XML (error='" << xmlError << ")" << endl;
  }

Download in other formats:

Plain Text