Both sides previous revision
Previous revision
Next revision
|
Previous revision
Next revision
Both sides next revision
|
dev:coding_style [2013/07/21 11:37] tedfelix Add headers |
dev:coding_style [2013/07/24 23:10] tedfelix Add assertions section. Formatting. |
| |
====== Rosegarden Coding Style ====== | ====== Rosegarden Coding Style ====== |
| |
| |
<code c++> | <code c++> |
if (something) somethingElse(); | if (something) somethingElse(); |
else someOtherThing(); | else someOtherThing(); |
if (something) { | |
somethingElse(); | if (something) { |
} else { | somethingElse(); |
someOtherThing(); | } else { |
} | someOtherThing(); |
| } |
</code> | </code> |
| |
| |
<code c++> | <code c++> |
if (something) | if (something) |
somethingElse(); | somethingElse(); |
</code> | </code> |
| |
Whitespace is much as in the above examples (outside but not inside ''()'', after but not before '';'', etc.): | Whitespace is much as in the above examples (outside but not inside ''()'', after but not before '';'', etc.): |
<code c++> | <code c++> |
connect(detailsButton, SIGNAL(clicked(bool)), this, SLOT(slotDetails())); | connect(detailsButton, SIGNAL(clicked(bool)), this, SLOT(slotDetails())); |
</code> | </code> |
| |
| |
<code c++> | <code c++> |
connect( detailsButton, SIGNAL( clicked(bool) ), this, SLOT( slotDetails() ) ); | connect( detailsButton, SIGNAL( clicked(bool) ), this, SLOT( slotDetails() ) ); |
</code> | </code> |
| |
| |
<code c++> | <code c++> |
if( something) | if( something) |
if(something) | if(something) |
</code> | </code> |
| |
| |
* 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: | * 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: |
| |
<code c++> | <code c++> |
connect(m_pluginList, SIGNAL(activated(int)), | connect(m_pluginList, SIGNAL(activated(int)), |
this, SLOT(slotPluginSelected(int))); | this, SLOT(slotPluginSelected(int))); |
</code> | </code> |
| |
| |
<code c++> | <code c++> |
connect(m_pluginList, SIGNAL(activated(int)), | connect(m_pluginList, SIGNAL(activated(int)), |
this, SLOT(slotPluginSelected(int))); | this, SLOT(slotPluginSelected(int))); |
</code> | </code> |
| |
| |
<code c++> | <code c++> |
CommandHistory::getInstance()->addCommand(new SegmentSyncCommand( | CommandHistory::getInstance()->addCommand(new SegmentSyncCommand( |
comp.getSegments(), selectedTrack, | comp.getSegments(), selectedTrack, |
dialog.getTranspose(), | dialog.getTranspose(), |
dialog.getLowRange(), | dialog.getLowRange(), |
dialog.getHighRange(), | dialog.getHighRange(), |
clefIndexToClef(dialog.getClef()))); | clefIndexToClef(dialog.getClef()))); |
</code> | </code> |
| |
| <code>//$$$</code> | Strings to change after a freeze ends | | | <code>//$$$</code> | 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 | * 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 <code>//</code> comments instead of C-style <code>/* */</code> comments (except as applies to writing comments specific to [[dev:doxygen|doxygen]].) | * When commenting out a large block of text, it is preferable to use C++ style <code>//</code> comments instead of C-style <code>/* */</code> comments (except as applies to writing comments specific to [[dev:doxygen|doxygen]].) |
| |
<code c++> | <code c++> |
//&&& This code was deleted temporarily | //&&& This code was deleted temporarily |
} */ | } */ |
</code> | </code> |
| |
(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.) | (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.) |
| |
Includes should try to follow the following pattern: | Includes should try to follow the following pattern: |
| |
<code> | <code c++> |
#include "MyHeader.h" | #include "MyHeader.h" |
| |
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 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: |
<code c++> | <code c++> |
switch (hfix) { | switch (hfix) { |
| |
case NoteStyle::Normal: | case NoteStyle::Normal: |
case NoteStyle::Reversed: | case NoteStyle::Reversed: |
if (params.m_stemGoesUp ^ (hfix == NoteStyle::Reversed)) { | if (params.m_stemGoesUp ^ (hfix == NoteStyle::Reversed)) { |
s0.setX(m_noteBodyWidth - stemThickness); | s0.setX(m_noteBodyWidth - stemThickness); |
} else { | } else { |
s0.setX(0); | 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; | |
} | } |
| break; |
| |
| case NoteStyle::Central: |
| if (params.m_stemGoesUp ^ (hfix == NoteStyle::Reversed)) { |
| s0.setX(m_noteBodyWidth / 2 + 1); |
| } else { |
| s0.setX(m_noteBodyWidth / 2); |
| } |
| break; |
| } |
</code> | </code> |
| |
| |
<code c++> | <code c++> |
switch(layoutMode) { | switch(layoutMode) { |
case 0 : | case 0 : |
findAction("linear_mode")->setChecked(true); | findAction("linear_mode")->setChecked(true); |
findAction("continuous_page_mode")->setChecked(false); | findAction("continuous_page_mode")->setChecked(false); |
findAction("multi_page_mode")->setChecked(false); | findAction("multi_page_mode")->setChecked(false); |
slotLinearMode(); | slotLinearMode(); |
break; | break; |
case 1 : | case 1 : |
findAction("linear_mode")->setChecked(false); | findAction("linear_mode")->setChecked(false); |
findAction("continuous_page_mode")->setChecked(true); | findAction("continuous_page_mode")->setChecked(true); |
findAction("multi_page_mode")->setChecked(false); | findAction("multi_page_mode")->setChecked(false); |
slotContinuousPageMode(); | slotContinuousPageMode(); |
break; | break; |
case 2 : | case 2 : |
findAction("linear_mode")->setChecked(false); | findAction("linear_mode")->setChecked(false); |
findAction("continuous_page_mode")->setChecked(false); | findAction("continuous_page_mode")->setChecked(false); |
findAction("multi_page_mode")->setChecked(true); | findAction("multi_page_mode")->setChecked(true); |
slotMultiPageMode(); | slotMultiPageMode(); |
break; | break; |
} | } |
</code> | </code> |
| |
===== Varables ===== | ===== 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: | * 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: |
| |
<code c++> | <code c++> |
int spacing = 3; | int spacing = 3; |
bool useHoops = false; | bool useHoops = false; |
| |
doSomething(spacing, useHoops); | doSomething(spacing, useHoops); |
</code> | </code> |
| |
| |
<code c++> | <code c++> |
doSomething(3, false); | doSomething(3, false); |
</code> | </code> |
| |
| |
<code c++> | <code c++> |
int i; | int i; |
int mi = -2; | int mi = -2; |
int md = getLineSpacing() * 2; | int md = getLineSpacing() * 2; |
| |
int testi = -2; | int testi = -2; |
int testMd = 1000; | int testMd = 1000; |
| |
for (i = -1; i <= 1; ++i) { | for (i = -1; i <= 1; ++i) { |
int d = y - getSceneYForHeight(ph + i, x, y); | int d = y - getSceneYForHeight(ph + i, x, y); |
if (d < 0) { | if (d < 0) { |
d = -d; | d = -d; |
} | |
if (d < md) { | |
md = d; | |
mi = i; | |
} | |
if (d < testMd) { | |
testMd = d; | |
testi = i; | |
} | |
} | } |
| if (d < md) { |
| md = d; |
| mi = i; |
| } |
| if (d < testMd) { |
| testMd = d; |
| testi = i; |
| } |
| } |
| </code> |
| |
| ===== Assertions ===== |
| Use Q_ASSERT_X() from <QtGlobal> for assertions. |
| |
| <code c++> |
| Q_ASSERT_X(id < m_refreshStatuses.size(), |
| "RefreshStatusArray::getRefreshStatus()", // where |
| "ID out of bounds"); // what |
| </code> |
| |
| Make sure errors are properly handled in a non-debug build. Do not depend on assertions. |
| |
| <code c++> |
| Q_ASSERT_X(p, "foo()", "null pointer"); |
| if (!p) { |
| return; |
| } |
</code> | </code> |
| |
For example: | For example: |
| |
<code> | <code xml> |
<Action name="general_move_events_up_staff" | <Action name="general_move_events_up_staff" |
text="&Move to Staff Above..." /> | text="&Move to Staff Above..." /> |
</code> | </code> |
| |