[vtk-developers] chaining vtk Set... methods

[Marcus D. Hanwell]

On Friday 29 January 2010 10:45:35 Michael Halle wrote:
> All this talk of vtkNew and vtkSmartPointer has got me thinking.
> 
> One of the elements of VTK syntax that has always bothered me is the
> redundancy of multiple "object->SetXXX();" method calls in typical
> applications.  You end up with a long list of "object->Set...;" lines.
>   That quite redundant. It's also error prone, since cutting and pasting
> code might result in an "object2->..." being added when "object->..."
> was intended.
> 
> The fix, as far as I know, is actually relatively simple (if not highly
> pervasive): change the return value of SetXXX() methods to be "this".

We have talked about this before. I would be in favor of it, but it is very 
pervasive as you said. One thing going for it is that backwards compatibility 
would not be broken (as you say below).
> 
> I don't know why I didn't think about it years ago, when the change
> would not have been as major.
> 
> // Compare (old way):
> 
>    vtkProperty *property = vtkProperty::New();
>    property->SetColor(1.0, 1.0, 1.0);
>    property->SetDiffuse(0.7);
>    property->SetSpecular(0.4);
>    property->SetSpecularPower(20);
> 
> // new way:
> 
>    vtkNew<vtkProperty> property;  // or old-style New() call
>    property->
>      SetColor(1.0, 1.0, 1.0)->
>      SetDiffuse(0.7)->
>      SetSpecular(0.4)->
>      SetSpecularPower(20);
> 
> // or
>    actor->GetProperty()->
>      SetColor(1.0, 1.0, 1.0)->
>      SetDiffuse(0.7)->
>      SetSpecular(0.4)->
>      SetSpecularPower(20);
> 
> 
> I believe the change to be backwards compatible (since SetXXX methods
> invariably return "void" now), and also compatible with the scripting
> wrappers.  You get natural indentation, reduced verbosity without loss
> of clarity, and the ability to pick and choose between old and new syntax.
> 
> Perhaps I missed a discussion about this idea sometime in the history of
> VTK, but are there technical downsides to this scheme?  The only major
> problem I see is that the new calling syntax can't be used with
> unmodified third party libraries that still return void instead of "this".
> 
> Insane?

I think it is sane, and a great pattern. That said, two revolutionary API 
changes in a week seems to be a little too much ;-) Also, it was pointed out 
to me when I discussed this last year that a large number of classes do not 
use the macro, and this would require manual modification. Is it acceptable to 
phase this in? If so how to we deal with inconsistent API?

I am not sure if there might be further issues associated with changing the 
function prototypes I am not aware of. As we do no maintain backwards ABI 
compatibility that would not be an issue.

Marcus
-- 
Marcus D. Hanwell, Ph.D.
R&D Engineer, Kitware Inc.
(518) 881-4937