[vtk-developers] Major problem in vtkRenderer.h

[Bill Hoffman]

Although it is done in many places in VTK, returning pointers to ivar's is a bad practice.  It really violates object encapsulation when you return a pointer to the internals of an object.     If you change one of these pointers, you can change the class itself and cause unpredictable results.  By using "const" this could be fixed, but right now that is not used much in VTK.   It is much safer to have the caller allocate the memory.

-Bill


At 10:45 AM 9/10/2002 -0400, Volpe, Christopher R (Research) wrote:
>Sorry about the mac bug. You're right, Lisa, I should have avoided the use of the static variable,
>particularly for thread safety reasons. I think Andy's solution is probably best. The solution below
>requires that the ComputeVisiblePropBound method be called six times in order to get all six boundary
>values. The extra class storage for a renderer ivar would be negligible (how many renderers does a
>typical application instantiate anyway, compared to, say, the number of Prop3D's that get created,
>which already have this variable.
>
>I'll check in a new version shortly...
>
>> Hello,
>> 
>> Even if this could work on the Mac it should not be in VTK. 
>> Since we can't 
>> change the fact that VTK returns pointers to arrays (although 
>> getting rid 
>> of that would be great!) one solution is to add a method 
>> something like this:
>> 
>> float GetVisibilePropBound( int index )
>> {
>>    float bounds[6];
>>    this->ComputeVisiblePropBounds(bounds);
>> 
>>    if ( index >= 0 && index <6 )
>>      {
>>      return bounds[index];
>>      }
>>    else
>>      {
>>      vtkErrorMacro("Bad index: " << index << " . Should be 0 to 5" );
>>      return 0.0;
>>      }
>> }
>> 
>> This method does not require any additional class storage, 
>> and is safe 
>> across multiple instances of a vtkRenderer even when running 
>> simultaneously 
>> in different threads. Probably the next best solution would 
>> be the one Andy 
>> suggested - create an ivar to store the returned value.  
>> static variable 
>> should be avoided at all costs.
>> 
>> Lisa
>> 
>> 
>> At 09:47 AM 9/10/2002, Andy Cedilnik wrote:
>> >Hello,
>> >
>> >Last night somebody added this to vtkRenderer.h. We need to 
>> get rid of
>> >this, since it does not work on Mac:
>> >
>> >   // Description:
>> >   // Wrapper-friendly version of ComputeVisiblePropBounds
>> >   float *ComputeVisiblePropBounds()
>> >     {
>> >     static float bounds[6];
>> >     this->ComputeVisiblePropBounds(bounds);
>> >     return bounds;};
>> >
>> >If there is another way, please use it. For example, create a member
>> >variable, and then do this:
>> >
>> >   float *ComputeVisiblePropBounds()
>> >     {
>> >     this->ComputeVisiblePropBounds(this->ComputedVisibleBounds);
>> >     return this->ComputedVisibleBounds;};
>> >
>> >                         Andy
>> >
>> >
>> >
>> >_______________________________________________
>> >vtk-developers mailing list
>> >vtk-developers at public.kitware.com
>> >http://public.kitware.com/mailman/listinfo/vtk-developers
>> 
>> _______________________________________________
>> vtk-developers mailing list
>> vtk-developers at public.kitware.com
>> http://public.kitware.com/mailman/listinfo/vtk-developers
>> 
>_______________________________________________
>vtk-developers mailing list
>vtk-developers at public.kitware.com
>http://public.kitware.com/mailman/listinfo/vtk-developers