VTK Coding Standards

General Coding Standards

We only have a few coding standards but they have proved very useful.

• We only put one public class per file. Some classes have helper classes that they use, but these are not accessible to the user.

• 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.

• Do not use default argument values for C++ method parameters. When the method is wrapped for tcl, the method appears with however many parameters it has and is impossible to call without specifying the parameter values anyway. Rather, use method overloading to achieve the same effect, even in tcl. With overloading, you can have the signature that has all the required parameters and signatures with extra parameters and have both be callable from C++ or tcl. The implementation of one signature should be in terms of the other signature with the default values for the parameters encoded in the .cxx file method implementation.

• Make sure your code compiles without any warnings with -Wall and -O2.

• 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 VTK_COMMON_EXPORT vtkBox : public vtkImplicitFunction
{
public:
  vtkTypeRevisionMacro(vtkBox,vtkImplicitFunction);
  void PrintSelf(ostream& os, vtkIndent indent);
...
}

• STL usage:
  • 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 heavily used superclasses. STL is big, fat, and slow to compile so we do not want to include STL headers in any .h files that are included by most of VTK, e.g. vtkObject.h vtkSource.h etc.
  • 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.
  • 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 <vtksys/stl/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.

• Do not use 'id' as a variable name in public headers as it is a reserved word in Objective-C++.

Common Pitfalls

Set method in constructor

Set methods defined with vtkSetMacro cannot be used in the default constructor to initialize ivars because the first line of a Set 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, in the constructor, the value of an ivar has to be initialized directly with the assignment operator not through a Set method defined with vtkSetMacro.

Example:

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
}

The issue looks pretty obvious in an isolated example like the one shown above. Sometimes it can also happen with an indirect call:

void vtkFoo::SetXToZero()
{
 this->SetX(0);
}

vtkFoo::vtkFoo
{
 this->SetXToZero(); // neh
}

---

VTK: [Welcome | Site Map]

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(ArrayTests 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*[])