Member visibility when writing new VTK classes

Development
1

As VTK tries to be as retrocompatible as possible with extensive deprecation mechanism, VTK developers often struggle with the amount of work needed to cleanly deprecate protected members (non-virtual methods and variables).

Indeed a protected members is perfectly accessible from a derived class, and VTK users have been deriving VTK class in their own apps for as long as VTK existed.

In order to remove a variable, change a name, change a method API, one need to cleanly deprecate it in order to be sure not to break someone else application.

The amount of work it represent can sometimes be overwhelming which means that sometimes, deprecation doesn’t happen, or the change that would be cleaner is not done to avoid deprecation.

A simple fix for that is to consider that members and non-virtual method should by default be put into private visibility, while only what should be considered the actual API of the class be put into public and protected visibility depending on the intent.

If will also make sure that setters and getters are correctly being defined and used in VTK, instead of accessing the members directly, which will improve our API.

Of course, all the existing code will not be moved, but new code should try to follow that and any rework of a class may want to deprecate protected members into private members if possible.

This is a suggestion, please let me know if you have any inputs on this.

Note: This is unrelated to the pimpl, which should already contain all internal member of the class.

2 Likes
2

Example, this should be avoided:

vtkMyClass
{
  protected:
    int myMember;
}

This should be prefered:

vtkMyClass
{
  public:
    vtkGetMacro(int, myMember);
    vtkSetMacro(int, myMember);

  private:
    int myMember;
}
1 Like
3

6 posts were split to a new topic: About avoiding public virtual method in VTK

9

Without any negative feedback, I will add that recommendation in the guide.

10

https://gitlab.kitware.com/vtk/vtk/-/merge_requests/10473