Rosegarden Coding Style

Rosegarden house style is as follows, in approximately descending order of importance:

Names

Class names are UpperCamelCase, method names are lowerCamelCase, non-public member variables are m_prefixedCamelCase, slot names start with “slot”.

Indentation - NO TAB CHARACTERS

Indentation is by four spaces (0x20) at a time. There should be no tab characters (0x09) anywhere in Rosegarden source code. The indentation should look the same regardless of whether you read it in an IDE, in a terminal window with “cat” or “vi”, in Emacs, or quoted in an email. It must not depend on having the right settings for tab-to-space conversion in your IDE when you read it. (Emacs and vim users will note that we already start every source file with a meta-comment that sets up the right indentation mode in these editors.)

NOTE: As of revision 13,509, Rosegarden has 8,691 tab characters in 287 files. Rosegarden 10.02 had 9,598 tab characters in 262 files. The good news is fewer tabs overall, but the bad news is 25 new files have tabs that never had tabs before. New developers are ignoring this rule, apparently.

Namespaces

No extra indentation inside namespaces; set off inner code by two carriage returns on either side of namespace brackets

namespace Rosegarden
{
// 1
// 2
class SomeUsefulClass
{
    //code
};
// 1
// 2
}

Braces

Braces are what you might call Java-style, which means:

int
SomeClass::someMethod(int f)
{
   int result = 0;
   if (f >= 0) {
       for (int i = 0; i < f; ++i) {
           result += i;
       }
   } else {
       result = -1;
   }
   return result;
}

if

Single statement blocks are preferably either “inline” or bracketed. Like this:

if (something) somethingElse();
else someOtherThing();
 
if (something) {
    somethingElse();
} else {
    someOtherThing();
}

but not:

if (something)
    somethingElse();

Parentheses and Whitespace

Whitespace is much as in the above examples (outside but not inside (), after but not before ;, etc.):

connect(detailsButton, SIGNAL(clicked(bool)), this, SLOT(slotDetails()));

but not:

connect( detailsButton, SIGNAL( clicked(bool) ), this, SLOT( slotDetails() ) );

and please avoid:

if( something)
if(something)

No whitespace between if (and other C++ keywords) and the ( makes Michael's eyes hurt.

Pointers

Pointers are MyObject *ptr; not MyObject* ptr;

Argument Alignment

If you have more arguments than will fit on a reasonable length line (80 characters is a good figure, but this is not a hard rule), align the extra arguments below and just after the opening ( rather than at a new level of indentation:

connect(m_pluginList, SIGNAL(activated(int)),
        this, SLOT(slotPluginSelected(int)));

but not:

connect(m_pluginList, SIGNAL(activated(int)),
    this, SLOT(slotPluginSelected(int)));

If your ( is so far to the right that following the above rule isn't practical, then indent the remainder of the () block by 8 spaces.

CommandHistory::getInstance()->addCommand(new SegmentSyncCommand(
        comp.getSegments(), selectedTrack,
        dialog.getTranspose(),
        dialog.getLowRange(),
        dialog.getHighRange(),
        clefIndexToClef(dialog.getClef())));

Comments

Use doxygen-style comments for class and function documentation

/** ... */

or

///

Use comment codes in your comments when appropriate:

Code Meaning
//!!!
TODO, or “this code is probably crap”
//&&&
Code temporarily disabled
//@@@
This code might not actually work
//###
Qt4 port info
//$$$
Strings to change after a freeze ends

Note: //!!! causes problems for doxygen. We might want to revise the above. Adding a space avoids problems: // !!!

If in doubt, please err on the side of putting in too many comments. We have contributors of all ability levels working here, and what may seem obvious to you might be complete gibberish to someone else. Comments give everyone a better chance to be useful, and they are always welcome, while nobody will think you are a super code warrior or code gazelle for committing a thousand lines that have only three choice comments

When commenting out a large block of text, it is preferable to use C++ style

//

comments instead of C-style

/* */

comments (except as applies to writing comments specific to doxygen.)

//&&& This code was deleted temporarily
//    for (int m = 0; m <= n; ++m) {
//        int x = s.x() + (int)((m * ((double)m * ax + bx)) / n);
//        int y = s.y() + (int)((m * ((double)m * ay + by)) / n);
//    }

but not:

//&&& This code was deleted temporarily
/*  for (int m = 0; m <= n; ++m) {
        int x = s.x() + (int)((m * ((double)m * ax + bx)) / n);
        int y = s.y() + (int)((m * ((double)m * ay + by)) / n);
    } */

(The reason C++-style comments are preferred is not arbitrary. C++-style comments are much more obvious when viewing diffs on the rosegarden-bugs list, because they force the entire block of text to be displayed, instead of only the starting and ending lines.)

#include Order

Includes should try to follow the following pattern (from local to global):

#include "MyHeader.h"

#include "MyCousinClassHeader.h"
#include "MyOtherCousin.h"

#include "RosegardenHeader.h"
#include "base/SomeOtherHeader.h"

#include <QSomeClass>
#include <QSomeOtherClass>

#include <some_stl_class>
#include <some_other_stl_class>

Switch

Switch statements are all over the place, and we need to pick one style and use it, so we'll call this a good switch statement:

switch (hfix) {
 
case NoteStyle::Normal:
case NoteStyle::Reversed:
    if (params.m_stemGoesUp ^ (hfix == NoteStyle::Reversed)) {
        s0.setX(m_noteBodyWidth - stemThickness);
    } else {
        s0.setX(0);
    }
    break;
 
case NoteStyle::Central:
    if (params.m_stemGoesUp ^ (hfix == NoteStyle::Reversed)) {
        s0.setX(m_noteBodyWidth / 2 + 1);
    } else {
        s0.setX(m_noteBodyWidth / 2);
    }
    break;
}

and this a bad one (note particularly the switch( here, grrrrr!):

switch(layoutMode) {
    case 0 :
        findAction("linear_mode")->setChecked(true);
        findAction("continuous_page_mode")->setChecked(false);
        findAction("multi_page_mode")->setChecked(false);
        slotLinearMode();
    break;
    case 1 :
        findAction("linear_mode")->setChecked(false);
        findAction("continuous_page_mode")->setChecked(true);
        findAction("multi_page_mode")->setChecked(false);
        slotContinuousPageMode();
    break;
    case 2 :
        findAction("linear_mode")->setChecked(false);
        findAction("continuous_page_mode")->setChecked(false);
        findAction("multi_page_mode")->setChecked(true);
        slotMultiPageMode();
    break;
}

Variables

You should feel encouraged to use variables to make the code easier to understand without a look at the header or the API docs. Good:

int spacing = 3;
bool useHoops = false;
 
doSomething(spacing, useHoops);

avoid:

doSomething(3, false);

Note that you should not feel like you have to take this to ridiculous extremes. Just bear it in mind, especially when it's something obscure enough you've just had to go look at the header or the API yourself to figure out what some static parameter was for.

Sometimes variables are simple and disposable, and names like i or n are appropriate, but unless your variable is something extremely simple and obvious, like an iterator, then it is preferable to use more verbose variable names in order to give the people who follow you a year later some shred of a hint what you were thinking when you wrote the code. This particularly egregious example does some rather complicated transformations that would have been crystal clear to the author at the time, but coming at it as a stranger, it's really just short of impossible to try to work out what the logic is supposed to do here. mi and ph and i oh my. It's just spectacularly impossible to maintain, so avoid doing this:

int i;
int mi = -2;
int md = getLineSpacing() * 2;
 
int testi = -2;
int testMd = 1000;
 
for (i = -1; i <= 1; ++i) {
    int d = y - getSceneYForHeight(ph + i, x, y);
    if (d < 0) {
        d = -d;
    }
    if (d < md) {
        md = d;
        mi = i;
    }
    if (d < testMd) {
        testMd = d;
        testi = i;
    }
}

Assertions

Use Q_ASSERT_X() from <QtGlobal> for assertions.

Q_ASSERT_X(id < m_refreshStatuses.size(),
           "RefreshStatusArray::getRefreshStatus()",  // where
           "ID out of bounds");                       // what

Make sure errors are properly handled in a non-debug build. Do not depend on assertions.

Q_ASSERT_X(p, "foo()", "null pointer");
if (!p) {
    return;
}

Menu Items

Menu items use title case. We don't capitalize pronouns except pronouns “standing alone” (exhaustive pronouns). For instance, “Move to Staff Above”.

We use trailing dots ”…” to indicate that a menu item leads to a dialog.

For example:

<Action name="general_move_events_up_staff" 
        text="&amp;Move to Staff Above..." />

See also:

 
 
dev/coding_style.txt · Last modified: 2013/10/18 00:38 by michael
Recent changes RSS feed Creative Commons License Valid XHTML 1.0 Valid CSS Driven by DokuWiki