[vtk-developers] Smart pointer declaration macro?

Thu Jan 28 08:50:43 EST 2010

Sorry, I don't like the abbreviation, I love change :-)

On Thu, Jan 28, 2010 at 8:45 AM, Jeff Baumes <jeff.baumes at kitware.com>wrote:

> 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
>

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