[vtk-developers] Smart pointer declaration macro?

[Jeff Baumes]

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