[vtk-developers] [coding tip] no call to Set in constructors

[Francois Bertel]

Thanks for clarifying and pointing out the case of strings and objects.

I actually said "cannot be used in the default constructor to
*initialize ivars*". I haven't say you couldn't use them in the
constructor for changing ivar values once they are initialized.

On Wed, Apr 29, 2009 at 9:40 AM, Moreland, Kenneth <kmorel at sandia.gov> wrote:
> A slight qualification: it is OK to call the Set function in the
> constructors _so long as you initialize the underlying variable first_.  I
> find I do this frequently with the string and object ivars to ensure that
> the memory management happens correctly.  For example:
>
> this->FileName = NULL;
> this->SetFileName("/tmp/log");
>
> or
>
> this->Controller = NULL;
> this->SetController(vtkMultiProcessController::GetGlobalController());
>
> Of course, setting the ivar to NULL first is vital, or things could
> potentially die a really horrible death.
>
> -Ken
>
>
> On 4/29/09 7:31 AM, "Francois Bertel" <francois.bertel at kitware.com> wrote:
>
> Hello,
>
> I've seen that more often in recent additions to VTK: calls to some
> Set methods in the default constructor.
>
> I would like to stress that Set methods defined with vtkSetMacro (it
> might be OK for other Set methods) 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
> }
>
> I added a section addressing this issue in the VTK Coding Standards:
> http://www.vtk.org/Wiki/VTK_Coding_Standards#Set_method_in_constructor
>
>
>
> Here is a real case in VTK, detected by valgrind:
>
> http://www.cdash.org/CDash/viewDynamicAnalysisFile.php?id=325444
>
> vtkRenderView::vtkRenderView() calls SetInteractionModeTo3D(), which
> calls SetInteractionMode(), but ivar InteractionMode has not been
> initialized yet.
>
>
>
> --
> François Bertel, PhD  | Kitware Inc. Suite 204
> 1 (518) 371 3971 x113 | 28 Corporate Drive
>                       | Clifton Park NY 12065, USA
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Follow this link to subscribe/unsubscribe:
> http://www.vtk.org/mailman/listinfo/vtk-developers
>
>
>
>
>
>    ****      Kenneth Moreland
>     ***      Sandia National Laboratories
> ***********
> *** *** ***  email: kmorel at sandia.gov
> **  ***  **  phone: (505) 844-8919
>     ***      web:   http://www.cs.unm.edu/~kmorel
>
>



-- 
François Bertel, PhD  | Kitware Inc. Suite 204
1 (518) 371 3971 x113 | 28 Corporate Drive
                      | Clifton Park NY 12065, USA