[vtk-developers] vtkImageConvolve -- why is it there?

[David Gobbi]

Hi Will,

Part of the reason for my initial disappointment was that when I
first saw that a vtkImageConvolve had been committed I thought
"Great!  An image convolution class!" but when I looked at the
source, it wasn't nearly as general as I had expected.

I'm a bit against 'extreme programming' unless it comes with
open & frequent communication.  If you tell the list that you
are committing a class (how long does that take?) then interested
people can glance at class and give feedback (and the feedback is
more likely to be positive than my feedback re. vtkImageConvolve).

I've found (and corrected) many bugs in VTK simply by reading
through the code, bugs that were not caught by testing
because the regression testing only catches _changes_ in
behaviour, it doesn't catch incorrect behaviour.  The more
eyes on the code, the better.

Another issue with 'extreme programming' (apart from the fact
that it is a buzzword and I HATE BUZZWORDS!) is that designing
things right the first time can save a lot of headaches later
(this isn't new, and is as true now as it ever was).
Thing will never 'automatically fall into place,' it is always
people putting things into the proper place.  So, adding new
code that hasn't been properly tested is only fine if an
equal (and perhaps greater) amount of time is spent auditing
existing code to ensure that APIs and behaviour are sensible.

Just my opinions.

 - David

--
  David Gobbi, MSc                    dgobbi at irus.rri.on.ca
  Advanced Imaging Research Group
  Robarts Research Institute, University of Western Ontario

On Thu, 24 May 2001, Will Schroeder wrote:

> Hi David-
>
> This was a class contributed by an outside party and sent to me.
> I threw it in the repository because I like to encourage contribution
> and I like to have the testing process chew on it to flesh out the bugs.
> As you can see it was renamed to vtkImageVectorConvolve because others
> had the same concerns that you did. The testing process also found some
> problems including the bad indexing. I tend to trust to the extreme
> programming process more (meaning poke something and see what emerges)
> rather than the cathedral approach of up front agreement on everything,
> which I know introduce chaos at time. I agree that certain additions
> need more discussion as this one did. I will definitely keep this in
> mind in the future.
>
> Will
>
> At 11:47 PM 5/21/2001 -0400, David Gobbi wrote:
> >I was looking through the CVS entries for today and there was
> >a new class, vtkImageConvolve, that doesn't look like it belongs.
> >
> >It convolves the image with a 3-element linear kernel (?) and is
> >therefore not general enough to be called 'vtkImageConvolve'
> >(a general convolution filter should accept an NxNxN kernel),
> >it computes the convolution incorrectly, and it has clearly
> >never been tested because the 'kernel' indices are in
> >the range [1,3] instead of [0,2] i.e. if it had been tested
> >it would have either given grossly incorrect results or would
> >have segfaulted.
> >
> >I've sent this plea out before, but I'll say it again.  Can't
> >we have some discussion on this list before new classes are
> >added or before major API changes are committed?
> >
> > - David