VTK Examples, modernising the code

Announcements
1

Just letting you all know that I am beginning a process of modernising the C++ VTK examples.
My initial approach will be to move from vtkSmartPointer to vtkNew where appropriate, use vtkNamedColors, use auto where appropriate, reformat, clean up code and finally make sure all tests work. I will be working through the examples folder by folder.

You will be able to follow progress here: web-test.
We still have to get the official web site up and working, that is work in progress.

3 Likes
2

Do we have some specific guidelines about the use of auto in VTK ?

3

As far as I know, no. My approach is that:

  1. Use auto where is a variable clearly int or double.
  2. In a for loop e.g. for( auto it = …; …; …)
  3. In cases like this:
auto pl3d_output =
      dynamic_cast<vtkStructuredGrid*>(pl3d->GetOutput()->GetBlock(0));

instead of:

vtkStructuredGrid* pl3d_output =
    dynamic_cast<vtkStructuredGrid*>(pl3d->GetOutput()->GetBlock(0));

Just use common sense e.g. http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-auto

4

To be consistent, VTK examples should follow VTK coding standards on where auto is encouraged/allowed/discouraged:

Use auto to avoid type names that are noisy, obvious, or unimportant - cases where the type doesn’t aid in clarity for the reader.

auto is permitted when it increases readability, particularly as described below. Never initialize an auto-typed variable with a braced initializer list.

Specific cases where auto is allowed or encouraged:

  • (Encouraged) For iterators and other long/convoluted type names, particularly when the type is clear from context (calls to find, begin, or end for instance).
  • (Allowed) When the type is clear from local context (in the same expression or within a few lines). Initialization of a pointer or smart pointer with calls to new commonly falls into this category, as does use of auto in a range-based loop over a container whose type is spelled out nearby.
  • (Allowed) When the type doesn’t matter because it isn’t being used for anything other than equality comparison.
  • (Encouraged) When iterating over a map with a range-based loop (because it is often assumed that the correct type is std::pair<KeyType, ValueType> whereas it is actually std::pair<const KeyType, ValueType>). This is particularly well paired with local key and value aliases for .first and .second (often const-ref).

for (const auto& item : some_map) {
const KeyType& key = item.first;
const ValType& value = item.second;
// The rest of the loop can now just refer to key and value,
// a reader can see the types in question, and we’ve avoided
// the too-common case of extra copies in this iteration.
}

  • (Discouraged) When iterating in integer space. for (auto i=0; i < grid->GetNumberOfPoints(); ++i). Because vtk data structures usually contain more than 2 billion elements, iterating using 32bit integer is discouraged (and often doesn’t match the type used)

In VTK core I would use auto carefully, as it makes code reviews harder (you need to spend time with figuring out what actual type of a variable is). But in examples, more use of auto could be acceptable, as making the code look simpler is very important.

SafeDownCast vs dynamic_cast was discussed some time ago. We need to be consistent and keep using SafeDownCast (unless somebody wants to reopen that discussion).

Maybe you could post here when you are done with the update of a first batch of examples so that you can get early feedback.

By the way, how do you maintain different language (C++, Python, Java) versions of the examples? Do you generate them automatically from a common version, or they are all handcrafted for each language?

2 Likes