[vtk-developers] Smart pointer declaration macro?

Thu Jan 28 09:30:43 EST 2010

Wes makes a good point. The other thing that is bothering me about

vtkSmartPointer<vtkClass> myLocal(true);

is that the form of this line of code, if you are not paying attention,
suggests that the class is being instantiated off of the stack rather than
heap. In general stack allocation is not allowed in VTK and I don't want
newbies getting the wrong idea.

On Thu, Jan 28, 2010 at 9:24 AM, Wes Turner <wes.turner at kitware.com> wrote:

> I'm not really against the idea, but I am not convinced that this actually
> does anything real for us.  We are just replacing the line:
>
> vtkSmartPointer<vtkClass> myLocal = vtkSmartPointer<vtkClass>::New();
>
> with
>
> vtkSmartPointer<vtkClass> myLocal(true);
>
> I don't mind change and will go along with whatever is decided, but we are
> trading conciseness for clarity.  I generally don't mind multiple line code,
> so shortening the signature is not a big deal to me.  Is there really
> consensus that this is needed?
>
> - Wes
>
> On Thu, Jan 28, 2010 at 9:05 AM, David Doria <daviddoria+vtk at gmail.com<daviddoria%2Bvtk at gmail.com>
> > wrote:
>
>> On Thu, Jan 28, 2010 at 9:01 AM, Will Schroeder
>> <will.schroeder at kitware.com> wrote:
>> > This has been part of the VTK naming convention as well, at least in
>> > practice.
>> >
>> > On Thu, Jan 28, 2010 at 8:54 AM, Karthik Krishnan
>> > <karthik.krishnan at kitware.com> wrote:
>> >>
>> >> Abbreviations in ITK are in the "things to avoid list" as far as
>> >> naming conventions go, unless the abbreviation is a standard one used
>> >> in the field such as FFT ..
>> >>
>> >> I suppose this is part of the VTK coding convention as well ? If not
>> >> it should probably be in there
>> >>
>> >> On Thu, Jan 28, 2010 at 8:50 AM, Will Schroeder
>> >> <will.schroeder at kitware.com> wrote:
>> >> > 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
>> >> >
>> >> > _______________________________________________
>> >> > 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
>> >> >
>> >> >
>> >> _______________________________________________
>> >> 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
>>
>> OK, so is everyone on board with Marcus' suggestion of:
>>
>> vtkSmartPointer<vtkClass> myLocal(true);
>>
>> If so, who is going to do it?
>>
>> Thanks,
>>
>> David
>> _______________________________________________
>> 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
>>
>>
>
>
> --
> Wesley D. Turner, Ph.D.
> Kitware, Inc.
> Technical Leader
> 28 Corporate Drive
> Clifton Park, NY 12065-8662
> Phone: 518-881-4920
>
> _______________________________________________
> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20100128/83e14982/attachment.html>