[vtk-developers] Smart pointer declaration macro?

[David Cole]

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20100127/bc61bb12/attachment.html>