|
|
| Line 1: |
Line 1: |
| ==General Coding Standards==
| | The VTK style guide has been moved to: |
| | | https://docs.google.com/a/kitware.com/document/d/1nzinw-dR5JQRNi_gb8qwLL5PnkGMK2FETlQGLr10tZw/edit |
| VTK is a large body of code with many users and developers. Coding in a consistent style eases shared development. VTK's style guidelines also ensure wide portability. All code that is contributed to VTK must conform to the following style guidelines. Exceptions are sometimes permissible, following discussion in code review, as long as the result passes the nightly regression tests. External code contributed into the ThirdParty directory, is exempt from most of the following rules except for the rules that say "All code".
| |
| | |
| * All code that is compiled into VTK by default must be compatible with VTK's BSD- style license.
| |
| * Copyright notices should appear at the top of header and implementation files.
| |
| | |
| * We only put one public class per file. Some classes have helper classes that they use, but these are not accessible to the user, and should employ the PIMPL idiom.
| |
| | |
| * Every class, macro, etc starts with either vtk or VTK, this avoids name clashes with other libraries. Classes should all start with vtk and macros or constants can start with either.
| |
| | |
| * Class names and file names are the same. This makes it easier to find the correct file for a specific class.
| |
| | |
| * We only use alphanumeric characters in names, [a-zA-z0-9]. So names like Extract_Surface are not welcome. We use capitalization to indicate words within a name. For example ExtractVectorTopology could be an instance variable. If it were a class it would be called vtkExtractVectorTopology. We capitalize the first letter of a name (excluding any preceding vtk). For local variables almost anything goes. Ideally we would suggest using same convention as instance variables except start their names with a lower case letter. e.g. extractVectorSurface.
| |
| | |
| * We try to always spell out a name and not use abbreviations. This leads to longer names but it makes using the software easier because you know that the SetRasterFontRange method will always be called that, not SetRFRange or SetRFontRange or SetRFR. When the name includes a natural abbreviation such as OpenGL, we keep the abbreviation and capitalize the abbreviated letters.
| |
| | |
| * We try to keep all instance variables protected. The user and application developer should access instance variables through Set/Get methods. To aid in this there are a number of macros defined in vtkSetGet.h that can be used. They expand into inline functions that Set/Get the instance variable and invoke a Modified() method if the value has changed.
| |
| | |
| * Use "this" inside of methods even though C++ doesn't require you to. This really seems to make the code more readable because it disambiguates between instance variables and local or global variables. It also disambiguates between member functions and other functions.
| |
| | |
| * Make sure your code compiles without any warnings with -Wall and -O2 (with gcc and clang).
| |
| | |
| * The indentation style can be characterized as the "indented brace" style. Indentations are two spaces, and the curly brace (scope delimiter) is placed on the following line and indented along with the code (i.e., the curly brace lines up with the code). Example:
| |
| | |
| if (this->Locator == locator)
| |
| {
| |
| return;
| |
| }
| |
| for (i = 0; i < this->Source->GetNumberOfPoints(); i++)
| |
| {
| |
| p1 = this->Source->GetPoint(i);
| |
| [...]
| |
| }
| |
| | |
| * The header file of the class should include only the superclass's header file. If you do not, the header test run as part of the VTK dashboard will report an error. If any other includes are absolutely necessary, include comment at each one describing why it should be included:
| |
| | |
| #include "vtkKWWindow.h"
| |
| #include "vtkClientServerID.h" // Needed for InteractorID
| |
| #include "vtkPVConfig.h" // Needed for PARAVIEW_USE_LOOKMARKS
| |
| | |
| * Avoid using vtkSetObjectMacro since it will require including the header file of another class. Use the vtkCxxSetObjectMacro instead. For example:
| |
| | |
| // Class declaration:
| |
| // Description:
| |
| // Set/Get the array used to store the visibility flags.
| |
| virtual void SetVisibilityById(vtkUnsignedCharArray* vis);
| |
| | |
| // Cxx file
| |
| vtkCxxSetObjectMacro(vtkStructuredVisibilityConstraint,
| |
| VisibilityById,
| |
| vtkUnsignedCharArray);
| |
| | |
| * All subclasses of vtkObject should include a PrintSelf() method that prints all publicly accessible ivars. For example:
| |
| | |
| void vtkObject::PrintSelf(ostream& os, vtkIndent indent)
| |
| {
| |
| os << indent << "Debug: " << (this->Debug ? "On\n" : "Off\n");
| |
| os << indent << "Modified Time: " << this->GetMTime() << "\n";
| |
| this->Superclass::PrintSelf(os, indent);
| |
| os << indent << "Registered Events: ";
| |
| if ( this->SubjectHelper )
| |
| {
| |
| os << endl;
| |
| this->SubjectHelper->PrintSelf(os,indent.GetNextIndent());
| |
| }
| |
| else
| |
| {
| |
| os << "(none)\n";
| |
| }
| |
| }
| |
| | |
| * All subclasses of vtkObject should include a type macro in their class declaration. For example:
| |
| | |
| class VTKCOMMONDATAMODEL_EXPORT vtkBox : public vtkImplicitFunction
| |
| {
| |
| public:
| |
| vtkTypeMacro(vtkBox, vtkImplicitFunction);
| |
| void PrintSelf(ostream& os, vtkIndent indent);
| |
| ...
| |
| }
| |
| | |
| * Do not use 'id' as a variable name in public headers as it is a reserved word in Objective-C++
| |
| | |
| * Long smart pointer definitions should break after the equals sign and have two spaces preceding the next line
| |
| | |
| vtkSmartPointer<vtkXMLPolyDataWriter> writer =
| |
| vtkSmartPointer<vtkXMLPolyDataWriter>::New();
| |
| | |
| * Prefer the use of vtkNew when the variable would be classically treated as a stack variable
| |
| | |
| vtkNew<vtkXMLPolyDataWriter> writer;
| |
| | |
| ===C++ standard ===
| |
| | |
| * VTK is written in C++03. Language/library features introduced in C++11 cannot be used.
| |
| * Code must be valid C++03 and C++11. (They are mostly backwards compatible, but not entirely. ex: the 'auto' keyword changed meaning in C++11.)
| |
| | |
| ===STL usage in VTK===
| |
| | |
| * STL usage in the Common modules (vtkCommonCore, vtkCommonDataModel etc):
| |
| ** STL is for implementation, not interface. STL references should be contained in a .cxx class or the private section of the .h header file
| |
| ** Use the PIMPL idiom to forward reference/contain STL classes in classes. Historically, STL was big, fat, and slow to compile so we do not want to include STL headers in any .h files that are included in public VTK header files in the vtkCommon modules
| |
| | |
| * STL usage is permitted in all non Common modules
| |
| ** It should be used sparingly where it makes sense and there is not an appropriate VTK class
| |
| ** std::string is now preferred over vtkStdString in API
| |
| ** STL use should use the PIMPL idiom if it does not add the to API/is not useful for inheritance
| |
| | |
| * General notes on STL use in VTK
| |
| ** Use the std:: namespace to refer to STL classes and functions (except for iostreams as mentioned below)
| |
| ** For an example of STL usage, see the [[VTK FAQ#Can I use STL with VTK?|VTK FAQ]].
| |
| ** If you do need to include STL files within a VTK header, you must add a comment after it so that the automated repository commit checks will accept your changes:
| |
| #include <vector> // STL Header <additional Comment here>
| |
| | |
| * When using anything declared in iostream, do not use std::. Examples include cerr, cout, ios...
| |
| ** Do not include the iostream header. It is already included
| |
| | |
| == Common Pitfalls ==
| |
| | |
| === Set method in constructor ===
| |
| | |
| <tt>Set</tt> methods defined with <tt>vtkSetMacro</tt> cannot be used in the default constructor to initialize ivars because the first line of a <tt>Set</tt> method is to compare the current value of the ivar with the value in argument. As at this point (in the constructor), the ivar is not initialized yet, the comparison happens against an uninitialized value.
| |
| | |
| valgrind will detect this kind of fault.
| |
| | |
| For this reason, <b>in the constructor, the value of an ivar has to be initialized directly with the assignment operator</b> not through a <tt>Set</tt> method defined with <tt>vtkSetMacro</tt>.
| |
| | |
| Example:
| |
| <pre>
| |
| class vtkFoo : public vtkObject
| |
| {
| |
| public:
| |
| ...
| |
| vtkGetMacro(X,int);
| |
| vtkSetMacro(X,int);
| |
| ...
| |
| protected:
| |
| vtkFoo();
| |
| int X;
| |
| ...
| |
| };
| |
| | |
| vtkFoo::vtkFoo
| |
| {
| |
| this->SetX(12); // neh
| |
| }
| |
| | |
| vtkFoo::vtkFoo
| |
| {
| |
| this->X=12; // good
| |
| }
| |
| </pre>
| |
| | |
| The issue looks pretty obvious in an isolated example like the one shown above. Sometimes it can also happen with an indirect call:
| |
| | |
| <pre>
| |
| void vtkFoo::SetXToZero()
| |
| {
| |
| this->SetX(0);
| |
| }
| |
| | |
| vtkFoo::vtkFoo
| |
| {
| |
| this->SetXToZero(); // neh
| |
| }
| |
| </pre>
| |
| | |
| | |
| {{VTK/Template/Footer}}
| |
| | |
| ==Documentation Standards==
| |
| ===Remaining questions===
| |
| * Where do default values go (with the set/get functions or with the variable definition?)
| |
| | |
| ===Standard Set/GetMacros===
| |
| The documentation style for this type of pair is very straight forward. There MUST be a single //Description for the pair and a brief description of the variable that is being set/get.
| |
| <source lang="cpp">
| |
| // Description:
| |
| // Set / get the sharpness of decay of the splats. This is the
| |
| // exponent constant in the Gaussian equation. Normally this is
| |
| // a negative value.
| |
| vtkSetMacro(ExponentFactor,double);
| |
| vtkGetMacro(ExponentFactor,double);
| |
| </source>
| |
| | |
| ===Set/GetVectorMacros===
| |
| The documentation style for vector macros is to name each of the resulting variables. For example
| |
| <source lang="cpp">
| |
| // Description:
| |
| // Set/Get DrawValue. This is the value that is used when filling data
| |
| // or drawing lines.
| |
| vtkSetVector4Macro(DrawColor, double);
| |
| vtkGetVector4Macro(DrawColor, double);
| |
| </source>
| |
| | |
| produces in the doxygen:
| |
| | |
| <source lang="cpp">
| |
| virtual void SetDrawColor (double, double, double, double)
| |
| </source>
| |
| | |
| We must therefore name the variables in the description, like
| |
| <source lang="cpp">
| |
| // Description:
| |
| // Set/Get the color which is used to draw shapes in the image. The parameters are SetDrawColor(red, green, blue, alpha)
| |
| vtkSetVector4Macro(DrawColor, double);
| |
| vtkGetVector4Macro(DrawColor, double);
| |
| </source>
| |
| | |
| ===Set/GetMacros + Boolean===
| |
| There must be a single description for this triple of macros. For example:
| |
| <source lang="cpp">
| |
| // Description:
| |
| // Turn on/off the generation of elliptical splats.
| |
| vtkSetMacro(NormalWarping,int);
| |
| vtkGetMacro(NormalWarping,int);
| |
| vtkBooleanMacro(NormalWarping,int);
| |
| </source>
| |
| | |
| ===SetClamp/GetMacros===
| |
| The description must describe the valid range of values.
| |
| <source lang="cpp">
| |
| // Description:
| |
| // Should the data with value 0 be ignored? Valid range (0, 1).
| |
| vtkSetClampMacro(IgnoreZero, int, 0, 1);
| |
| vtkGetMacro(IgnoreZero, int);
| |
| </source>
| |
| | |
| == Testing Standards ==
| |
| | |
| * When adding a new concrete class, a test should also be added in
| |
| ...VTK/Package/Testing/Cxx/
| |
| | |
| * The name of the file for the test should be ClassName.cxx where the name of the class is vtkClassName.
| |
| | |
| * Each test should call several functions, each as short as possible, to exercise a specific functionality of the class. 1,000 lines in main() is very hard to maintain...
| |
| | |
| * The "main()" function of the test file must be called TestClassName(int, char*[])
| |
| | |
| ===Questions===
| |
| * Do we bother covering deprecated classes?
| |
| * Is there a way to mark deprecated classes as deprecated so they do not show up on the coverage dashboards?
| |
| * What is our target coverage percentage? 70%?
| |
| * How to test abstract classes?
| |
| * Can we create a system that checks for the existence of a test file for every concrete class? This would be a great place to start to improve coverage.
| |
| * When should a new "test prefix" be started? By adding tests to a list like this:
| |
| | |
| CREATE_TEST_SOURCELIST(''Array''Tests ArrayCxxTests.cxx
| |
| | |
| they show up when running ctest as:
| |
| | |
| Start 33: ''Array''-TestArrayBool
| |
| | |
| * If arguments are not required, should you use
| |
| TestName(int vtkNotUsed(argc), char *vtkNotUsed(argv)[])
| |
| or
| |
| TestName(int, char*[])
| |
