[vtk-developers] Smart pointer declaration macro?

[Jeff Baumes]

On Wed, Jan 27, 2010 at 10:58 AM, Will Schroeder
<will.schroeder at kitware.com> wrote:
> VTK has a verbose, self documenting style (for better or worse). I'd like to
> stick with it if possible.

Will,

Are you suggesting no change? Or just that you don't like the
abbreviated typedefs like vtkRendererSP?

Jeff

>
>
> On Wed, Jan 27, 2010 at 10:56 AM, David Cole <david.cole at kitware.com> wrote:
>>
>> On Wed, Jan 27, 2010 at 8:35 AM, Jeff Baumes <jeff.baumes at kitware.com>
>> wrote:
>>>
>>> On Tue, Jan 26, 2010 at 12:44 PM, Marcus D. Hanwell
>>> <marcus.hanwell at kitware.com> wrote:
>>> > On Friday 08 January 2010 16:46:47 David Cole wrote:
>>> >> On Fri, Jan 8, 2010 at 4:08 PM, Marcus D. Hanwell <
>>> >>
>>> >> marcus.hanwell at kitware.com> wrote:
>>> >> > I would like to add my -1, I think needing to split lines in order
>>> >> > to
>>> >> > declare
>>> >> > a new local variable is a little much. I came from a C++ background
>>> >> > where
>>> >> > any
>>> >> > object could be declared on the stack though. For things like the
>>> >> > examples it
>>> >> > seems to hurt readability to me.
>>> >> >
>>> >> > Pointer:
>>> >> > vtkFloatArray *myTable = vtkFloatArray::New();
>>> >> > myTable->Delete();
>>> >> > myTable = NULL;
>>> >> >
>>> >> > Smart pointer:
>>> >> > vtkSmartPointer<vtkFloatArray> myTable =
>>> >> >     vtkSmartPointer<vtkFloatArray>::New();
>>> >> >
>>> >> > Smart pointer with macro:
>>> >> > VTK_CREATE(vtkFloatArray, myTable);
>>> >> >
>>> >> > Stack:
>>> >> > vtkFloatArray myTable;
>>> >> >
>>> >> > I would prefer to be able to use something like the first or the
>>> >> > last. In
>>> >> > classes etc it is often a different story. It seems like there
>>> >> > should be
>>> >> > some
>>> >> > macro or template function to generate variables with less
>>> >> > repetition.
>>> >>
>>> >> Prefer the first and last as much as you want, but we simply can't use
>>> >> them
>>> >> in VTK. The first leads to memory leaks because people forget the
>>> >> Delete
>>> >> calls. The last cannot be done with vtkObject derived classes because
>>> >> of
>>> >>  the nature of vtkObject reference counting...
>>> >>
>>> > Wasn't suggesting either (just pointing out the shorter syntax that
>>> > people
>>> > miss), although the first is still widely used in VTK.
>>> >
>>> >> So we have to pick one of the middle ones...
>>> >>
>>> >> It's unfortunate that we've had two +1's and two -1's... that leaves
>>> >> us at
>>> >>  0 for the moment. I guess that and the fact that it's Friday makes it
>>> >>  fairly easy to put off a decision until at least next week. ;-)
>>> >>
>>> >> *If* we do go with a macro-based approach, I think we can all agree
>>> >> there
>>> >> should be one centralized macro that does this and it should be used
>>> >> *everywhere* that vtkSmartPointer::New is presently used.
>>> >
>>> > What about a vtkLocalPointer<vtkClass> myLocal; where the default
>>> > constructor
>>> > makes an instance on the VTK class? It would also be possible to have a
>>> > constructor take an argument (may be a little clunkier) such as
>>> > vtkSmartPointer<vtkClass> myLocal(true); if we do not want to introduce
>>> > yet
>>> > another class.
>>> >
>>> > Alternate options a and b...
>>> >
>>> > a) vtkLocalPointer<vtkClass> myLocal;
>>> > b) vtkSmartPointer<vtkClass> myLocal(true);
>>> >
>>> > Would this be preferable to a macro? It seems like a better way to go
>>> > to me,
>>> > and in terms of API and conciseness seems to satisfy our requirements
>>> > better
>>> > than the current approach. It would still be possible to share the
>>> > pointer
>>> > too, due to the reference counting in vtkObject derived classes.
>>>
>>> I think option b is a good option. It seems that it would be odd to
>>> have two different pointer classes, where the only difference is what
>>> they do on construction.
>>>
>>> The biggest grudge I have about the current option is that it almost
>>> always requires a split line if you constrain yourself to 80
>>> characters. If your type name is more than about 15 characters (which
>>> many VTK types are), you have to split your line:
>>>
>>> vtkSmartPointer<type> name = vtkSmartPointer<type>::New();
>>> 16 + t + 2 + n + 19 + t + 9 = 46 + 2*t + n
>>>
>>> Option b, even though not the most concise, would make it easy to stay
>>> within that limit, about halving the number of characters:
>>>
>>> vtkSmartPointer<type> name(true);
>>> 16 + t + 2 + n + 7 = 25 + t + n
>>>
>>> Jeff
>>
>>
>> Another solution worth considering is simply making a typedef called
>> "vtkRendererSP" that is a typedef for "vtkSmartPointer<vtkRenderer>" -- that
>> would allow you to:
>> vtkRendererSP renderer = vtkRendererSP::New();
>> It gives you the shorter names you're all longing for, without changing
>> anything else already in VTK...
>>
>> David C.
>>
>> _______________________________________________
>> 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
>>
>>
>
>
>
> --
> William J. Schroeder, PhD
> Kitware, Inc.
> 28 Corporate Drive
> Clifton Park, NY 12065
> will.schroeder at kitware.com
> http://www.kitware.com
> (518) 881-4902
>



-- 
Jeff Baumes, Ph.D.
R&D Engineer, Kitware Inc.
(518) 881-4932
jeff.baumes at kitware.com