I will say that you should avoid "Set" methods entirely from constructors unless you verify the following:<div>-- the Set method in question is *not* *virtual*<br><br></div><div>If the Set method is virtual and you call it from a constructor of a class that's mid-way through a deeply nested hierarchy of classes (it has parent classes and sub-classes defined) then you are trying to do something that is probably not what you expect. A C++ object should only initialize its own classes ivars (or possibly some from its parent classes if it has to give different default values for some of those) in the constructor. Nothing else. Anything else should be done after the construction of the object is complete in a 2nd pass "initialize(with, these, params);" type of call.</div>
<div><br></div><div>You are inviting unexplained behavior to occur (eventually, for someone) if you call virtual methods from the constructor.</div><div><br></div><div>It has always seemed to me that there should be a C++ compiler warning issued when a virtual method is called from a constructor.</div>
<div><br></div><div><br></div><div>Other opinions welcome...</div><div>:-)</div><div>David Cole</div><div><br></div><div><br><div class="gmail_quote">On Wed, Apr 29, 2009 at 9:52 AM, Francois Bertel <span dir="ltr"><<a href="mailto:francois.bertel@kitware.com">francois.bertel@kitware.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">Thanks for clarifying and pointing out the case of strings and objects.<br>
<br>
I actually said "cannot be used in the default constructor to<br>
*initialize ivars*". I haven't say you couldn't use them in the<br>
constructor for changing ivar values once they are initialized.<br>
<div><div></div><div class="h5"><br>
On Wed, Apr 29, 2009 at 9:40 AM, Moreland, Kenneth <<a href="mailto:kmorel@sandia.gov">kmorel@sandia.gov</a>> wrote:<br>
> A slight qualification: it is OK to call the Set function in the<br>
> constructors _so long as you initialize the underlying variable first_. I<br>
> find I do this frequently with the string and object ivars to ensure that<br>
> the memory management happens correctly. For example:<br>
><br>
> this->FileName = NULL;<br>
> this->SetFileName("/tmp/log");<br>
><br>
> or<br>
><br>
> this->Controller = NULL;<br>
> this->SetController(vtkMultiProcessController::GetGlobalController());<br>
><br>
> Of course, setting the ivar to NULL first is vital, or things could<br>
> potentially die a really horrible death.<br>
><br>
> -Ken<br>
><br>
><br>
> On 4/29/09 7:31 AM, "Francois Bertel" <<a href="mailto:francois.bertel@kitware.com">francois.bertel@kitware.com</a>> wrote:<br>
><br>
> Hello,<br>
><br>
> I've seen that more often in recent additions to VTK: calls to some<br>
> Set methods in the default constructor.<br>
><br>
> I would like to stress that Set methods defined with vtkSetMacro (it<br>
> might be OK for other Set methods) cannot be used in the default<br>
> constructor to initialize ivars because the first line of a Set method<br>
> is to compare the current value of the ivar with the value in<br>
> argument. As at this point (in the constructor), the ivar is not<br>
> initialized yet, the comparison happens against an uninitialized<br>
> value.<br>
><br>
> valgrind will detect this kind of fault.<br>
><br>
> For this reason, in the constructor, the value of an ivar has to be<br>
> initialized directly with the assignment operator not through a Set<br>
> method defined with vtkSetMacro.<br>
><br>
> Example:<br>
><br>
> class vtkFoo : public vtkObject<br>
> {<br>
> public:<br>
> ...<br>
> vtkGetMacro(X,int);<br>
> vtkSetMacro(X,int);<br>
> ...<br>
> protected:<br>
> vtkFoo();<br>
> int X;<br>
> ...<br>
> };<br>
><br>
> vtkFoo::vtkFoo<br>
> {<br>
> this->SetX(12); // neh<br>
> }<br>
><br>
> vtkFoo::vtkFoo<br>
> {<br>
> this->X=12; // good<br>
> }<br>
><br>
> The issue looks pretty obvious in an isolated example like the one<br>
> shown above. Sometimes it can also happen with an indirect call:<br>
><br>
> void vtkFoo::SetXToZero()<br>
> {<br>
> this->SetX(0);<br>
> }<br>
><br>
> vtkFoo::vtkFoo<br>
> {<br>
> this->SetXToZero(); // neh<br>
> }<br>
><br>
> I added a section addressing this issue in the VTK Coding Standards:<br>
> <a href="http://www.vtk.org/Wiki/VTK_Coding_Standards#Set_method_in_constructor" target="_blank">http://www.vtk.org/Wiki/VTK_Coding_Standards#Set_method_in_constructor</a><br>
><br>
><br>
><br>
> Here is a real case in VTK, detected by valgrind:<br>
><br>
> <a href="http://www.cdash.org/CDash/viewDynamicAnalysisFile.php?id=325444" target="_blank">http://www.cdash.org/CDash/viewDynamicAnalysisFile.php?id=325444</a><br>
><br>
> vtkRenderView::vtkRenderView() calls SetInteractionModeTo3D(), which<br>
> calls SetInteractionMode(), but ivar InteractionMode has not been<br>
> initialized yet.<br>
><br>
><br>
><br>
> --<br>
> François Bertel, PhD | Kitware Inc. Suite 204<br>
> 1 (518) 371 3971 x113 | 28 Corporate Drive<br>
> | Clifton Park NY 12065, USA<br>
> _______________________________________________<br>
> Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
><br>
> Visit other Kitware open-source projects at<br>
> <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
><br>
> Follow this link to subscribe/unsubscribe:<br>
> <a href="http://www.vtk.org/mailman/listinfo/vtk-developers" target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br>
><br>
><br>
><br>
><br>
><br>
> **** Kenneth Moreland<br>
> *** Sandia National Laboratories<br>
> ***********<br>
> *** *** *** email: <a href="mailto:kmorel@sandia.gov">kmorel@sandia.gov</a><br>
> ** *** ** phone: (505) 844-8919<br>
> *** web: <a href="http://www.cs.unm.edu/~kmorel" target="_blank">http://www.cs.unm.edu/~kmorel</a><br>
><br>
><br>
<br>
<br>
<br>
</div></div>--<br>
<div><div></div><div class="h5">François Bertel, PhD | Kitware Inc. Suite 204<br>
1 (518) 371 3971 x113 | 28 Corporate Drive<br>
| Clifton Park NY 12065, USA<br>
_______________________________________________<br>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://www.vtk.org/mailman/listinfo/vtk-developers" target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br>
<br>
</div></div></blockquote></div><br></div>