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:<div><br></div><div><div>vtkSmartPointer<vtkClass> myLocal = vtkSmartPointer<vtkClass>::New();</div>
<div><br></div><div>with</div><div><br></div><div><div>vtkSmartPointer<vtkClass> myLocal(true);</div><div><br></div><div>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?</div>
<div><br></div><div>- Wes</div><div><br></div></div></div><div><div class="gmail_quote">On Thu, Jan 28, 2010 at 9:05 AM, David Doria <span dir="ltr"><<a href="mailto:daviddoria%2Bvtk@gmail.com">daviddoria+vtk@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">On Thu, Jan 28, 2010 at 9:01 AM, Will Schroeder<br>
<div><div></div><div class="h5"><<a href="mailto:will.schroeder@kitware.com">will.schroeder@kitware.com</a>> wrote:<br>
> This has been part of the VTK naming convention as well, at least in<br>
> practice.<br>
><br>
> On Thu, Jan 28, 2010 at 8:54 AM, Karthik Krishnan<br>
> <<a href="mailto:karthik.krishnan@kitware.com">karthik.krishnan@kitware.com</a>> wrote:<br>
>><br>
>> Abbreviations in ITK are in the "things to avoid list" as far as<br>
>> naming conventions go, unless the abbreviation is a standard one used<br>
>> in the field such as FFT ..<br>
>><br>
>> I suppose this is part of the VTK coding convention as well ? If not<br>
>> it should probably be in there<br>
>><br>
>> On Thu, Jan 28, 2010 at 8:50 AM, Will Schroeder<br>
>> <<a href="mailto:will.schroeder@kitware.com">will.schroeder@kitware.com</a>> wrote:<br>
>> > Sorry, I don't like the abbreviation, I love change :-)<br>
>> ><br>
>> ><br>
>> > On Thu, Jan 28, 2010 at 8:45 AM, Jeff Baumes <<a href="mailto:jeff.baumes@kitware.com">jeff.baumes@kitware.com</a>><br>
>> > wrote:<br>
>> >><br>
>> >> On Wed, Jan 27, 2010 at 10:58 AM, Will Schroeder<br>
>> >> <<a href="mailto:will.schroeder@kitware.com">will.schroeder@kitware.com</a>> wrote:<br>
>> >> > VTK has a verbose, self documenting style (for better or worse). I'd<br>
>> >> > like to<br>
>> >> > stick with it if possible.<br>
>> >><br>
>> >> Will,<br>
>> >><br>
>> >> Are you suggesting no change? Or just that you don't like the<br>
>> >> abbreviated typedefs like vtkRendererSP?<br>
>> >><br>
>> >> Jeff<br>
>> >><br>
>> >> ><br>
>> >> ><br>
>> >> > On Wed, Jan 27, 2010 at 10:56 AM, David Cole <<a href="mailto:david.cole@kitware.com">david.cole@kitware.com</a>><br>
>> >> > wrote:<br>
>> >> >><br>
>> >> >> On Wed, Jan 27, 2010 at 8:35 AM, Jeff Baumes<br>
>> >> >> <<a href="mailto:jeff.baumes@kitware.com">jeff.baumes@kitware.com</a>><br>
>> >> >> wrote:<br>
>> >> >>><br>
>> >> >>> On Tue, Jan 26, 2010 at 12:44 PM, Marcus D. Hanwell<br>
>> >> >>> <<a href="mailto:marcus.hanwell@kitware.com">marcus.hanwell@kitware.com</a>> wrote:<br>
>> >> >>> > On Friday 08 January 2010 16:46:47 David Cole wrote:<br>
>> >> >>> >> On Fri, Jan 8, 2010 at 4:08 PM, Marcus D. Hanwell <<br>
>> >> >>> >><br>
>> >> >>> >> <a href="mailto:marcus.hanwell@kitware.com">marcus.hanwell@kitware.com</a>> wrote:<br>
>> >> >>> >> > I would like to add my -1, I think needing to split lines in<br>
>> >> >>> >> > order<br>
>> >> >>> >> > to<br>
>> >> >>> >> > declare<br>
>> >> >>> >> > a new local variable is a little much. I came from a C++<br>
>> >> >>> >> > background<br>
>> >> >>> >> > where<br>
>> >> >>> >> > any<br>
>> >> >>> >> > object could be declared on the stack though. For things like<br>
>> >> >>> >> > the<br>
>> >> >>> >> > examples it<br>
>> >> >>> >> > seems to hurt readability to me.<br>
>> >> >>> >> ><br>
>> >> >>> >> > Pointer:<br>
>> >> >>> >> > vtkFloatArray *myTable = vtkFloatArray::New();<br>
>> >> >>> >> > myTable->Delete();<br>
>> >> >>> >> > myTable = NULL;<br>
>> >> >>> >> ><br>
>> >> >>> >> > Smart pointer:<br>
>> >> >>> >> > vtkSmartPointer<vtkFloatArray> myTable =<br>
>> >> >>> >> > vtkSmartPointer<vtkFloatArray>::New();<br>
>> >> >>> >> ><br>
>> >> >>> >> > Smart pointer with macro:<br>
>> >> >>> >> > VTK_CREATE(vtkFloatArray, myTable);<br>
>> >> >>> >> ><br>
>> >> >>> >> > Stack:<br>
>> >> >>> >> > vtkFloatArray myTable;<br>
>> >> >>> >> ><br>
>> >> >>> >> > I would prefer to be able to use something like the first or<br>
>> >> >>> >> > the<br>
>> >> >>> >> > last. In<br>
>> >> >>> >> > classes etc it is often a different story. It seems like there<br>
>> >> >>> >> > should be<br>
>> >> >>> >> > some<br>
>> >> >>> >> > macro or template function to generate variables with less<br>
>> >> >>> >> > repetition.<br>
>> >> >>> >><br>
>> >> >>> >> Prefer the first and last as much as you want, but we simply<br>
>> >> >>> >> can't<br>
>> >> >>> >> use<br>
>> >> >>> >> them<br>
>> >> >>> >> in VTK. The first leads to memory leaks because people forget<br>
>> >> >>> >> the<br>
>> >> >>> >> Delete<br>
>> >> >>> >> calls. The last cannot be done with vtkObject derived classes<br>
>> >> >>> >> because<br>
>> >> >>> >> of<br>
>> >> >>> >> the nature of vtkObject reference counting...<br>
>> >> >>> >><br>
>> >> >>> > Wasn't suggesting either (just pointing out the shorter syntax<br>
>> >> >>> > that<br>
>> >> >>> > people<br>
>> >> >>> > miss), although the first is still widely used in VTK.<br>
>> >> >>> ><br>
>> >> >>> >> So we have to pick one of the middle ones...<br>
>> >> >>> >><br>
>> >> >>> >> It's unfortunate that we've had two +1's and two -1's... that<br>
>> >> >>> >> leaves<br>
>> >> >>> >> us at<br>
>> >> >>> >> 0 for the moment. I guess that and the fact that it's Friday<br>
>> >> >>> >> makes<br>
>> >> >>> >> it<br>
>> >> >>> >> fairly easy to put off a decision until at least next week. ;-)<br>
>> >> >>> >><br>
>> >> >>> >> *If* we do go with a macro-based approach, I think we can all<br>
>> >> >>> >> agree<br>
>> >> >>> >> there<br>
>> >> >>> >> should be one centralized macro that does this and it should be<br>
>> >> >>> >> used<br>
>> >> >>> >> *everywhere* that vtkSmartPointer::New is presently used.<br>
>> >> >>> ><br>
>> >> >>> > What about a vtkLocalPointer<vtkClass> myLocal; where the default<br>
>> >> >>> > constructor<br>
>> >> >>> > makes an instance on the VTK class? It would also be possible to<br>
>> >> >>> > have a<br>
>> >> >>> > constructor take an argument (may be a little clunkier) such as<br>
>> >> >>> > vtkSmartPointer<vtkClass> myLocal(true); if we do not want to<br>
>> >> >>> > introduce<br>
>> >> >>> > yet<br>
>> >> >>> > another class.<br>
>> >> >>> ><br>
>> >> >>> > Alternate options a and b...<br>
>> >> >>> ><br>
>> >> >>> > a) vtkLocalPointer<vtkClass> myLocal;<br>
>> >> >>> > b) vtkSmartPointer<vtkClass> myLocal(true);<br>
>> >> >>> ><br>
>> >> >>> > Would this be preferable to a macro? It seems like a better way<br>
>> >> >>> > to<br>
>> >> >>> > go<br>
>> >> >>> > to me,<br>
>> >> >>> > and in terms of API and conciseness seems to satisfy our<br>
>> >> >>> > requirements<br>
>> >> >>> > better<br>
>> >> >>> > than the current approach. It would still be possible to share<br>
>> >> >>> > the<br>
>> >> >>> > pointer<br>
>> >> >>> > too, due to the reference counting in vtkObject derived classes.<br>
>> >> >>><br>
>> >> >>> I think option b is a good option. It seems that it would be odd to<br>
>> >> >>> have two different pointer classes, where the only difference is<br>
>> >> >>> what<br>
>> >> >>> they do on construction.<br>
>> >> >>><br>
>> >> >>> The biggest grudge I have about the current option is that it<br>
>> >> >>> almost<br>
>> >> >>> always requires a split line if you constrain yourself to 80<br>
>> >> >>> characters. If your type name is more than about 15 characters<br>
>> >> >>> (which<br>
>> >> >>> many VTK types are), you have to split your line:<br>
>> >> >>><br>
>> >> >>> vtkSmartPointer<type> name = vtkSmartPointer<type>::New();<br>
>> >> >>> 16 + t + 2 + n + 19 + t + 9 = 46 + 2*t + n<br>
>> >> >>><br>
>> >> >>> Option b, even though not the most concise, would make it easy to<br>
>> >> >>> stay<br>
>> >> >>> within that limit, about halving the number of characters:<br>
>> >> >>><br>
>> >> >>> vtkSmartPointer<type> name(true);<br>
>> >> >>> 16 + t + 2 + n + 7 = 25 + t + n<br>
>> >> >>><br>
>> >> >>> Jeff<br>
>> >> >><br>
>> >> >><br>
>> >> >> Another solution worth considering is simply making a typedef called<br>
>> >> >> "vtkRendererSP" that is a typedef for "vtkSmartPointer<vtkRenderer>"<br>
>> >> >> --<br>
>> >> >> that<br>
>> >> >> would allow you to:<br>
>> >> >> vtkRendererSP renderer = vtkRendererSP::New();<br>
>> >> >> It gives you the shorter names you're all longing for, without<br>
>> >> >> changing<br>
>> >> >> anything else already in VTK...<br>
>> >> >><br>
>> >> >> David C.<br>
>> >> >><br>
>> >> >> _______________________________________________<br>
>> >> >> Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
>> >> >><br>
>> >> >> Visit other Kitware open-source projects at<br>
>> >> >> <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
>> >> >><br>
>> >> >> Follow this link to subscribe/unsubscribe:<br>
>> >> >> <a href="http://www.vtk.org/mailman/listinfo/vtk-developers" target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br>
>> >> >><br>
>> >> >><br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> > --<br>
>> >> > William J. Schroeder, PhD<br>
>> >> > Kitware, Inc.<br>
>> >> > 28 Corporate Drive<br>
>> >> > Clifton Park, NY 12065<br>
>> >> > <a href="mailto:will.schroeder@kitware.com">will.schroeder@kitware.com</a><br>
>> >> > <a href="http://www.kitware.com" target="_blank">http://www.kitware.com</a><br>
>> >> > (518) 881-4902<br>
>> >> ><br>
>> >><br>
>> >><br>
>> >><br>
>> >> --<br>
>> >> Jeff Baumes, Ph.D.<br>
>> >> R&D Engineer, Kitware Inc.<br>
>> >> (518) 881-4932<br>
>> >> <a href="mailto:jeff.baumes@kitware.com">jeff.baumes@kitware.com</a><br>
>> ><br>
>> ><br>
>> ><br>
>> > --<br>
>> > William J. Schroeder, PhD<br>
>> > Kitware, Inc.<br>
>> > 28 Corporate Drive<br>
>> > Clifton Park, NY 12065<br>
>> > <a href="mailto:will.schroeder@kitware.com">will.schroeder@kitware.com</a><br>
>> > <a href="http://www.kitware.com" target="_blank">http://www.kitware.com</a><br>
>> > (518) 881-4902<br>
>> ><br>
>> > _______________________________________________<br>
>> > Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
>> ><br>
>> > Visit other Kitware open-source projects at<br>
>> > <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
>> ><br>
>> > Follow this link to subscribe/unsubscribe:<br>
>> > <a href="http://www.vtk.org/mailman/listinfo/vtk-developers" target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br>
>> ><br>
>> ><br>
>> _______________________________________________<br>
>> Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
>><br>
>> Visit other Kitware open-source projects at<br>
>> <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
>><br>
>> Follow this link to subscribe/unsubscribe:<br>
>> <a href="http://www.vtk.org/mailman/listinfo/vtk-developers" target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br>
>><br>
><br>
><br>
><br>
> --<br>
> William J. Schroeder, PhD<br>
> Kitware, Inc.<br>
> 28 Corporate Drive<br>
> Clifton Park, NY 12065<br>
> <a href="mailto:will.schroeder@kitware.com">will.schroeder@kitware.com</a><br>
> <a href="http://www.kitware.com" target="_blank">http://www.kitware.com</a><br>
> (518) 881-4902<br>
<br>
</div></div>OK, so is everyone on board with Marcus' suggestion of:<br>
<br>
vtkSmartPointer<vtkClass> myLocal(true);<br>
<br>
If so, who is going to do it?<br>
<br>
Thanks,<br>
<font color="#888888"><br>
David<br>
</font><div><div></div><div class="h5">_______________________________________________<br>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://www.vtk.org/mailman/listinfo/vtk-developers" target="_blank">http://www.vtk.org/mailman/listinfo/vtk-developers</a><br>
<br>
</div></div></blockquote></div><br><br clear="all"><br>-- <br>Wesley D. Turner, Ph.D.<br>Kitware, Inc.<br>Technical Leader<br>28 Corporate Drive<br>Clifton Park, NY 12065-8662<br>Phone: 518-881-4920<br>
</div>