Documentation/Nightly/Developers/Style Guide
For the latest Slicer documentation, visit the read-the-docs. |
Code style
Code that inherits from VTK classes should follow VTK coding conventions for naming, indentation, etc. See http://www.vtk.org/Wiki/VTK_Coding_Standards for details.
Naming
- Acronyms should be written with the same case for each letter (all uppercase or all lowercase).
- RASToSlice not RasToSlice
- vtkMRML not vtkMrml
- vtkSlicer not vTKSlicer
- Words should be spelled out and not abreviated
- GetWindow not GetWin
- File names must follow the Camel case convention
- TestMyFeature.cxx not Test-My_Feature.cxx
- Use US English words and spelling
- "Millimeter" not "Millimetre"
- "Color" not "Colour"
Comments
- include extensive comments in the header files
- keep the comments up to date as the code changes
- use the keyword
\todo
to flag spots in the code that need to be revisited - do not leave blocks of commented out code in the source file -- if needed insert links to prior svn versions as comments
Functions
- Don't mix different levels of abstraction
- Examples:
- When dealing with files names and path, use kwsys::SystemTools in VTK classes, QFileInfo/QDir in Qt classes or os.path in python. Instead of doing string manipulation manually:
QString filePath = directoryPath + "/" + fileName + ".exe"
)
- prefer instead:
SystemTools::JoinPath(), SystemTools::GetFilenameName()...
QFileInfo(QDir directory, QString fileName), QFileInfo::suffix(), QFileInfo::absoluteFilePath()...
os.path.join(), os.path.splitext(), os.path.abspath()...
- When dealing with files names and path, use kwsys::SystemTools in VTK classes, QFileInfo/QDir in Qt classes or os.path in python. Instead of doing string manipulation manually:
- References:
- Clean Code from Robert C. Martin: Mixing levels of abstraction within a function is always confusing. Readers may not be able to tell whether a particular expression is an essential concept or a detail. Worse, like broken windows, once details are mixed with essential concepts, more and more details tend to accrete within the functions.
- http://zuskin.com/coding_habits__functions.htm#Abstraction_levels
- Examples:
- Use STL where you can, but follow the VTK guidelines
- However, prefer Qt Container classes to STL classes in Qt files
- Note that a vtkCollection is somewhat equivalent to
std::list<vtkSmartPointer<vtkObject*> >
Layout
Includes
- Only include the necessary files, no more.
- Group includes per library
- Alphabetically sort files within groups
- Order groups from local to global
- e.g. Module then MRML then CTK then Qt then VTK then ITK then STL
- Implementation files should include the header files first
For example:
// header // ... // end header #include "qSlicerMyModule.h" // MyModule includes #include "qSlicerMyModuleWidget.h" #include "vtkSlicerMyModuleLogic.h" // MRML includes #include "vtkMRMLScene.h" // Qt includes #include <QDialog> // VTK includes #include <vtkSmartPointer.h> // STD includes #include <vector>
Language Specific
Library Dependencies
- MRML classes should only depend on vtk and itk (not Slicer Logic or Qt)
- Logic classes depend on MRML to store state
- Logic classes should encapsulate vtk and itk pipelines to accomplish specific slicer tasks (such as resampling volumes for display)
- GUI classes can depend on MRML and Logic and Qt
Development Practices
- While developing code, enable VTK_DEBUG_LEAKS (ON by default) in your vtk build and be sure to clean up any leaks that arise from your contributions.
Coordinate Systems
- World space for 3D Views is in RAS (Right Anterior Superior) space. See Coordinate systems.
- All units are expressed in Millimeters (mm)
Error and warning messages
The ITK, VTK, Qt, std::cout, std::cerr .. all appear in the error log and can easily be filtered according to their type (debug/warning/error).
|
- In Qt-based classes:
- For error messages, use qCritical().
if (somethingWrongHappened) { qCritical() << "I encountered an error"; return; }
- For warnings, use qWarning().
qWarning() << "Be careful here, this is dangerous";
- For debug, use qDebug():
qDebug() << "This variable has the value: "<< value;
- In VTK-based classes:
- For error messages, use vtkErrorMacro().
if (somethingWrongHappened) { vtkErrorMacro("I encountered an error"); return; }
- For warnings, use vtkWarningMacro().
vtkWarningMacro("Be careful here, this is dangerous");
- For debug, use vtkDebugMacro():
vtkDebugMacro("This variable has the value: "<< value);
Misc.
- No more than 80 characters per line
Commits
Summary
- Prefix the commit message title with "BUG:","ENH:","COMP:", "STYLE:". Note the ':' (colon) character.
- Explain in the commit message title the "impact of the commit to the user"
- Add in the commit message body the Mantis issue number prefixed by '#' (pound) character.
Commit message prefix
Subversion Commits to Slicer require commit type in the comment.
Valid commit types are:
BUG: - a change made to fix a runtime issue (crash, segmentation fault, exception, or incorrect result, COMP: - a fix for a compilation issue, error or warning, ENH: - new functionality added to the project, PERF: - a performance improvement, STYLE: - a change that does not impact the logic or execution of the code. (improve coding style, comments, documentation).
Note that the ':'(colon) directly follows the commit tag. For example, it is: "STYLE:" not "STYLE :"
The Subversion command to commit the change is:
svn commit -m "BUG: fixed core dump when passed float data" filename1[, filename2, ...]
By using the -m
command line option, it's not possible to submit a message having multiple line.
Submitting a mutli-line message can be achieved using the -f
option:
svn commit -f /path/to/message filename1[, filename2, ...]
It's also possible to set the environment variable SVN_EDITOR
Message content
- A good commit message title (first line) should explain what the commit does for the user, not how it is done. How can be explained in the body of the commit message (if looking at the code of the commit is not self explanatory enough).
- Examples:
- Bad:
BUG: Check pointer validity before dereferencing
-> implementation detail, self-explanatory (by looking at the code) - Good:
BUG: Fix crash in Module X when clicking Apply button
- Bad:
ENH: More work in qSlicerXModuleWidget
->more work
is too vague,qSlicerXModuleWidget
is too low level - Good:
ENH: Add float image outputs in module X
- Bad:
COMP: Typo in cmake variable
-> implementation detail, self-explanatory - Good:
COMP: Fix compilation error with Numpy on Visual Studio
- Bad:
- Examples:
- If the commit is related to a mantis issue (bug or feature request), you can mention it in the commit message body by preceding the issue number with a #(pound) character:
BUG: Fix crash in Volume Rendering module when switching view layout vtkSetAndObserveMRMLNodeEventsMacro can't be used for observing all types of vtkObjects, only vtkMRMLNode is expected by vtkMRMLAbstractLogic::OnMRMLNodeModified(...) Closes #1641
Where 1641
refers to the issue number in mantis.
- Notice the empty 2nd line.
Importing changes from external project/repository
When you update the git tag or svn revision of any external project, explicit in the commit message what the update does instead of just mentioning that an update in made.
This will avoid having a Slicer commit history made of ineligible messages:
r19180 - ENH: Update git tag r19181 - BUG: Update svn revision r19182 - ENH: revision updated ...
Ideally it should be the same message than the commit(s) in the external repository.
Example:
ENH: Add feature A in module X
instead of ENH: Update git tag
Resources
http://www.na-mic.org/Wiki/index.php/Engineering:Subversion_Repository
http://www.slicer.org/slicerWiki/index.php/Slicer:git-svn
UI Design Guidelines
General guidelines
- As a general rule, follow those following guidelines:
Panels
Section
A section is used in a panel to categorize parameters by visually grouping them. In the Volume Rendering module, there are 3 sections: 'Inputs', 'Display', and 'Advanced...'. By default, the 'Inputs' and 'Advanced...' sections are collapsed. This reduces visual cluttering by hiding advanced and rarely used parameters. Sections should be organized in such a way that the workflow takes the user from top to bottom:
- The 'Inputs' section is first as it controls the inputs of the volume rendering.
- then the 'Display' section controls important display properties
- finally, if the previous parameters are not enough to obtain the desired result, the Advanced.. section offers fine tuning of the volume rendering.
Please note that the Advanced-ness of a section doesn't necessarily impacts its position in the section ordering. To create a section you must use a ctkCollapsibleButton [1] with no panel frame. Typically, the main node selector (Volume: in Volume Rendering) is the first GUI element and is outside any section.
Parameters
Elements in panels should be justified (use of a QFormLayout can simplify the task)
Text
- Capitalize the first letter in any text specified for a label or button:
- "Load volumes" instead of "Load Volumes"
- More capitalization rules
- Try to use brief phrases when specifying text for a label or button rather than using sentences or sentence fragments ( use "Load volumes" instead of "Choose a volume to load")
- Provide fully descriptive tool tips with each widget defined
- Don't use colon after each parameter labels:
- "Load volumes" instead of "Load volumes:"
- Use US English and avoid abbreviations (see the Naming section above.)
Layouts
- Use the default values for the margins or 0. Default margins are automatically controlled by the Slicer custom style (see QStyle::PM_LayoutLeftMargin)
- The minimum size hint of the top level module widget is used to determine the minimum width of the module. The minimum width of a module must not be larger than 500px (subject to change). This is enforced by the automatic test qSlicerMY_MODULE_NAMEModuleGenericTest. If the width is too large, you have multiple ways of narrowing the module panel:
- In Qt Designer, you can ensure the sizing is correct by changing the QLayout::SizeConstraint to
QLayout::SetMinimumSize
. When you preview the module (Ctrl-R), it appears with the same size as it would in Slicer. - Ideally the minimum size hint of each wide GUI element should be fixed, it is unlikely that a large size hint is "ideal".
- Alternatively, you might want to investigate the following:
- Consider setting the horizontal size policy of such element to QSizePolicy::Ignored
- If the element is in a form layout, QFormLayout::layoutFieldGrowthPolicy might need to be set to
AllNonFixedFieldsGrow
. - All those changes can be set from Qt Designer.
- In Qt Designer, you can ensure the sizing is correct by changing the QLayout::SizeConstraint to
Links
Logging
In VTK classes:
- vtkErrorMacro("vtkMRMLClipModelsNode:: Invalid Clip Type");
- vtkWarningMacro("Model " << modelNode->GetName() << "'s display node is null\n");
- vtkDebugMacro("CreateWidget: found a glyph type already defined for this node: " << iter->second);
In QT classes:
- qCritical() << "qSlicerUtils::setPermissionsRecursively: Failed to set permissions on file" << info.filePath();
- qWarning() << "qSlicerIOManager::openScreenshotDialog: Unable to get Annotations module (annotations), using the CTK screen shot dialog.";
- qDebug() << "qMRMLSceneFactoryWidget::deleteNode(" <<className <<") no node";
Wiki
- Prefer to follow Wikipedia conventions for page naming: For multiword page titles, leave the second and subsequent words in lowercase unless the title phrase is a proper noun that would always occur capitalized, even in the middle of a sentence.
- Versioned pages should include '<noinclude>{{documentation/versioncheck}}</noinclude>' at the very beginning.