[vtk-developers] Smart pointer declaration macro?

Fri Jan 8 16:27:14 EST 2010

So, I will vote -1 on Bill's comment.  Normally I'm all for extra typing for the sake of clarity.  For example, I'm quite adamant about using longer descriptive variable and class names, avoiding the use of initialisms, and inserting comments where the code is not self explanatory.  However, in this case I think the extra typing serves the opposite case: It hides the intentions of the code.

The intension of the smart pointer is for it to behave like a regular pointer.  The entire class is design to hide the complexity of managing references.  Details like pointer dereferencing and type casting are hidden under the covers.  Lots of things are happening under the covers in that little "->", but you don't care.  By thinking about the symbol as a simple pointer you can focus better on what the code is supposed to do.

Where the vtkSmartPointer does a bad job of looking like a pointer is in declaration and instantiation.  Ideally, I want them to look the same as (or as close as possible to) a regular pointer:

vtkPoints *instance = vtkPoints::New();

When I have a line like

vtkSmartPointer<vtkPoints> instance = vtkSmartPointer<vtkPoints>::New();

it (1) is no longer obvious I am declaring, creating, and initializing the pointer, (2) makes it hard to see at a glance the salient bits of informations (the object type, the variable name, and the object creation), and (3) it almost always spans multiple lines which makes it harder to view the rest of the code.  This is why I frequently use those VTK_CREATE macros (and have been since vtkSmartPointer was created).

VTK_CREATE(vtkPoints, instance);

The meaning is clear and you can pick out the salient information at a glance.  This might all be nit-picking, but it can have a dramatic effect on code readability.

I've been using macros because it is the easiest way to add a feature like this without changing some of the base classes (for which we weak mortals are unworthy).  If you hate macros, fine.  There are good reasons to.  I have no large objections to the template/typedef/constructor solutions below.  But there should be something.

-Ken

On 1/8/10 1:54 PM, "Bill Lorensen" <bill.lorensen at gmail.com> wrote:

And VTK_CREATE? And who knows what others...

On Fri, Jan 8, 2010 at 3:52 PM, David Cole <david.cole at kitware.com> wrote:
> On Fri, Jan 8, 2010 at 3:41 PM, David Gobbi <david.gobbi at gmail.com> wrote:
>>
>> On Fri, Jan 8, 2010 at 1:38 PM, David Doria <daviddoria+vtk at gmail.com>
>> wrote:
>> > On Fri, Jan 8, 2010 at 3:33 PM, Karthik Krishnan
>> > <karthik.krishnan at kitware.com> wrote:
>> >> Or have these as typdefs and/or factory methods in a VTK class, as is
>> >> done in ITK (that would be internally handled via macros in the
>> >> class), so that usage syntax looks like.
>> >>
>> >>  vtkPoints::SmartPointer = vtkPoints::SmartNew();
>> >> or
>> >>  vtkPoints::SmartPointer instance(1);
>> >>
>> >>
>> >>
>> >>
>> >>
>> >> On Fri, Jan 8, 2010 at 3:26 PM, David Gobbi <david.gobbi at gmail.com>
>> >> wrote:
>> >>> On Fri, Jan 8, 2010 at 1:15 PM, David Doria <daviddoria+vtk at gmail.com>
>> >>> wrote:
>> >>>> I've seen the use of something like this:
>> >>>>
>> >>>> #define SPNEW(instance, type) \
>> >>>> vtkSmartPointer<type> instance = vtkSmartPointer<type>::New();
>> >>>>
>> >>>> in many of the tests and elsewhere. It seems like a reasonable
>> >>>> savings
>> >>>> of a whole bunch of characters that appears many many times in most
>> >>>> functions. Could we standardize something like this so it can be used
>> >>>> universally without having to see this little #define in every file
>> >>>> it
>> >>>> is used in?
>> >>>>
>> >>>> Thoughts?
>> >>>
>> >>> Doesn't VTK have enough macros already? ;)
>> >>>
>> >>> There are other ways to reduce the repetition, without using macros:
>> >>>
>> >>> 1) Add a new constructor argument for smart pointers:
>> >>>
>> >>> vtkSmartPointer<type> instance(1); // create an smart pointer and
>> >>> allocate an object at the same time
>> >>> vtkSmartPointer<type> instance(0); // create a smart pointer with
>> >>> "null" as the initial pointer
>> >>>
>> >>> 2) Add a non-static "New" method for smart pointers:
>> >>>
>> >>> vtkSmartPointer<type> instance;
>> >>> instance.InstantiateNew();
>> >>>
>> >>> etc.
>> >>>
>> >>>   David
>> >
>> >
>> > Sure, I hate macros :)
>> >
>> > So how do we turn these good suggestions into a final conclusion and
>> > course of action?
>> >
>> > My choice/thought after seeing these initial comments is that it would
>> > be nice if ITK and VTK shared a similar style (the ::SmartPointer).
>> >
>> > Bill - there is plenty of mystery even with the additional typing :)
>> >
>> > David D.
>>
>> Actually, I'll give a "+1" to Bill's comment because I do like code to
>> be as explicit as possible.
>>
>>  David G
>>
>
> Me too. +1.
> Which sort of means eliminating all the existing SPNEW macros from test
> files, doesn't it...? :-)
>
> 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
>
>
>
_______________________________________________
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

   ****      Kenneth Moreland
    ***      Sandia National Laboratories
***********
*** *** ***  email: kmorel at sandia.gov
**  ***  **  phone: (505) 844-8919
    ***      web:   http://www.cs.unm.edu/~kmorel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/vtk-developers/attachments/20100108/702bb3f4/attachment.html>