[vtk-developers] vtk-developers Digest, Vol 158, Issue 21

[Elvis Stansvik]

2017-06-23 10:33 GMT+02:00 Elvis Stansvik <elvis.stansvik at orexplore.com>:
> Sorry, accidently hit send. Fixes below.
>
> 2017-06-23 10:29 GMT+02:00 Elvis Stansvik <elvis.stansvik at orexplore.com>:
>> 2017-06-23 1:21 GMT+02:00 Andrew Maclean <andrew.amaclean at gmail.com>:
>>> Hi Bill, Elvis,
>>>    Elvis, personally I wouldn't like to see the homogenisation of the
>>> examples by doing what you propose.
>>> The reasons are:
>>> 1) One of the advantages of the examples is seeing the different approaches
>>> used by the contributors.
>>> 2) It may dissuade contributors by implicitly forcing them to use a
>>> particular approach.
>>> 3) One of the really useful things in the example is the different ways VTK
>>> is used.
>>
>> I absolutely agree with 1 and 3 (which I think are the same?), but I
>> don't see how changing to auto would in affect anything in this
>> regard.
>>
>> I also don't see how it would be a homogenization. The declarations I
>> would change are already homogeneous in that they're all
>> vtkSmartPointer<Foo> a = vtkSmartPointer<Foo>::New(). Changing to auto
>> would not make it more or less homogeneous.
>>
>> It would be a
>
> ... It would be homogenisation if I'd change all
> vtkNew/vtkSmartPointer to auto a = vtkSmartPointer..., but that's not
> what this is about.
>
>> Note that this is not about changing vtkNew to vtkSmartPointer.
>>
>> And how would changing to auto in any way affect the approach taken by
>> the example?
>>
>>
>>
>>> To me it matters little whether:
>>> auto actor = vtkSmartPointer<vtkActor>::New();
>>> vtkSmartPointer<vtkActor> actor =
>>>     vtkSmartPointer<vtkActor>::New();
>>>
>>> or whether "ren/renWin" is used instead of "renderer" or "rendererWindow" in
>>> the examples.
>
> It matters little to me too, but it does matter. I think it's almost
> indisputable that
>
>     auto someVar = vtkSmartPointer<SomeLongTypeName>::New()
>
> is more readable than
>
>     vtkSmartPointer<SomeLongTypeName> someVar =
>         vtkSmartPointer<SomeLongTypeName>::New();
>
> especially since the latter leads to many more lines to scan across
> when looking for something in the examples.

Another small plus I see with using auto is it's a keyword which would
be highlighted, so the declarations would stand out more.

E.g. looking at the code for this example

    https://lorensen.github.io/VTKExamples/site/Cxx/Filtering/ConnectivityFilter/

I find it hard to quickly answer "what are the declarations?", since
they have no highlighting compared to the surrounding statements. Had
they been auto, it would have been easier since I think auto would
have been highlighted.

I think quickly identifying the variables involved helps the reading
of the examples.

Elvis

>
> So, in short I agree with everything you say, but I can't see how
> changing one way of doing declarations to another is a homogenization.
> And I do think spelling matters.
>
> I'm perfectly OK with leaving the examples exactly like they are
> though, just wanted to explain how I see it.
>
> Elvis
>
>>>
>>> Of more importance are explanatory notes in the examples.
>>>
>>> Bill, I see you are using vtkNamedColors. This example shows what other
>>> things you can do with this class:
>>> https://lorensen.github.io/VTKExamples/site/Cxx/Visualization/NamedColors/
>>>
>>> For example, assign colors by name:
>>> renderer->SetBackground(namedColors->GetColor3d("SteelBlue").GetData
>>> ());
>>> Create your own named color (in this case a red with an alpha of 0.5):
>>> namedColors->GetColor("Red", rgba);
>>> rgba[3] = 0.5; namedColors->SetColor("My Red", rgba);
>>>
>>> Regards
>>>    Andrew
>>>
>>>>
>>>> ---------- Forwarded message ----------
>>>> From: Bill Lorensen <bill.lorensen at gmail.com>
>>>> To: Elvis Stansvik <elvis.stansvik at orexplore.com>
>>>> Cc: vtkdev <vtk-developers at vtk.org>
>>>> Bcc:
>>>> Date: Thu, 22 Jun 2017 13:32:55 -0400
>>>> Subject: Re: [vtk-developers] vtkNew in examples (or auto?)
>>>> Let's leave them as is for now. I want to make sure I understand this.
>>>>
>>>>
>>>> On Thu, Jun 22, 2017 at 1:13 PM, Elvis Stansvik
>>>> <elvis.stansvik at orexplore.com> wrote:
>>>> > 2017-06-22 19:09 GMT+02:00 Bill Lorensen <bill.lorensen at gmail.com>:
>>>> >> I'm not sure what you mean by auto-fying
>>>> >
>>>> > Sorry, I should have been clearer. What I mean is changing declarations
>>>> > such as
>>>> >
>>>> >
>>>> vtkSmartPointer<vtkActor> actor =
>>>> >     vtkSmartPointer<vtkActor>::New();
>>>> >
>>>> > into
>>>> >
>>>> >   auto actor = vtkSmartPointer<vtkActor>::New();
>>>> >
>>>> > I think it would cut down on the number of lines in many examples, and
>>>> > make them more readable. (This would only be done in places where the
>>>> > type of the variable is still clear from the declaration.)
>>>> >
>>>> > Elvis
>>>> >
>>>> >>
>>>> >>
>>>> >>
>>>> >> On Thu, Jun 22, 2017 at 1:07 PM, Elvis Stansvik
>>>> >> <elvis.stansvik at orexplore.com> wrote:
>>>> >>> 2017-06-22 19:01 GMT+02:00 Bill Lorensen <bill.lorensen at gmail.com>:
>>>> >>>> I prefer vtkSmartPointer because it can be used like any other vtk
>>>> >>>> pointer. No need for a GetPointer() is some cases. The example writer
>>>> >>>> is free to use vtkSmartPointer or vtkNew. But I would leave them as
>>>> >>>> there are.
>>>> >>>
>>>> >>> Right, that's a valid point. But how about auto-fying the
>>>> >>> declarations? (but keep using vtkSmartPointer)
>>>> >>>
>>>> >>> My motivation is that when reading an example, I'm often squinting to
>>>> >>> find the variable names in the declarations, wedged in there somewhere
>>>> >>> between all those type names and angle brackets. Especially as the
>>>> >>> lines are often broken due to running long.
>>>> >>>
>>>> >>>>
>>>> >>>>
>>>> >>>> Other cleanup's sound great. I've also started using vtkNamedColors
>>>> >>>> instead of setting float values.
>>>> >>>
>>>> >>> Great.
>>>> >>>
>>>> >>> Elvis
>>>> >>>
>>>> >>>>
>>>> >>>> On Thu, Jun 22, 2017 at 12:57 PM, Elvis Stansvik
>>>> >>>> <elvis.stansvik at orexplore.com> wrote:
>>>> >>>>> Hi all,
>>>> >>>>>
>>>> >>>>> How about a refactor of the examples to use vtkNew instead of
>>>> >>>>> vtkSmartPointer (where it makes sense)?
>>>> >>>>>
>>>> >>>>> E.g.
>>>> >>>>>
>>>> >>>>>   vtkNew<vtkActor> actor;
>>>> >>>>>   actor->SetMapper(mapper);
>>>> >>>>>
>>>> >>>>>   vtkNew<vtkRenderer> renderer;
>>>> >>>>>   renderer->AddActor(actor);
>>>> >>>>>
>>>> >>>>> instead of
>>>> >>>>>
>>>> >>>>>   vtkSmartPointer<vtkActor> actor =
>>>> >>>>>     vtkSmartPointer<vtkActor>::New();
>>>> >>>>>   actor->SetMapper(mapper);
>>>> >>>>>
>>>> >>>>>   vtkSmartPointer<vtkRenderer> renderer =
>>>> >>>>>     vtkSmartPointer<vtkRenderer>::New();
>>>> >>>>>   renderer->AddActor(actor);
>>>> >>>>>
>>>> >>>>> I think it would help with the readability of the examples. Or are
>>>> >>>>> there other reasons for the prevalent use of vtkSmartPointer?
>>>> >>>>>
>>>> >>>>> Another option would be to use auto, e.g.
>>>> >>>>>
>>>> >>>>>   auto actor = vtkSmartPointer<vtkActor>::New();
>>>> >>>>>
>>>> >>>>> Also, would anyone mind if I did a little naming cleanup, mostly
>>>> >>>>> things like "renwin" -> "window" and "iren" -> "interactor"? Those
>>>> >>>>> abbreviations are not that bad, but I think it's better in examples
>>>> >>>>> to
>>>> >>>>> spell out the variables in proper English.
>>>> >>>>>
>>>> >>>>> If there are no objections, I could try to prepare an MR when time
>>>> >>>>> permits. If so, vtkNew, or auto?
>>>> >>>>>
>>>> >>>>> Elvis
>>>> >>>>> _______________________________________________
>>>> >>>>> Powered by www.kitware.com
>>>> >>>>>
>>>> >>>>> Visit other Kitware open-source projects at
>>>> >>>>> http://www.kitware.com/opensource/opensource.html
>>>> >>>>>
>>>> >>>>> Search the list archives at:
>>>> >>>>> http://markmail.org/search/?q=vtk-developers
>>>> >>>>>
>>>> >>>>> Follow this link to subscribe/unsubscribe:
>>>> >>>>> http://public.kitware.com/mailman/listinfo/vtk-developers
>>>> >>>>>
>>>> >>>>
>>>> >>>>
>>>> >>>>
>>>> >>>> --
>>>> >>>> Unpaid intern in BillsBasement at noware dot com
>>>> >>
>>>> >>
>>>> >>
>>>> >> --
>>>> >> Unpaid intern in BillsBasement at noware dot com
>>>>
>>>>
>>>>
>>>> --
>>>> Unpaid intern in BillsBasement at noware dot com
>>>>
>>>
>>>
>>> --
>>> ___________________________________________
>>> Andrew J. P. Maclean
>>>
>>> ___________________________________________