[vtk-developers] comments inside BTX ETX + matrix variable naming

[David Doria]

On Sat, Dec 12, 2009 at 10:32 AM, Marcus D. Hanwell
<marcus.hanwell at kitware.com> wrote:
> On Sat, Dec 12, 2009 at 10:23 AM, David Doria <daviddoria+vtk at gmail.com>
> wrote:
>>
>> On Sat, Dec 12, 2009 at 10:17 AM, Francois Bertel
>> <francois.bertel at kitware.com> wrote:
>> > Arguments of functions and methods in VTK start with lowercase. So
>> > it's is not OK. Only the variable member of a class start with an
>> > upper case.
>>
>> It seems very annoying to not comply with mathematics notation, which
>> is clearly more standard than VTK notation.
>>
> The mathematical notation is there to make it clear that the variable in the
> formula is a matrix, this is C++ code in a matrix class, which already makes
> it clear that these variables are concerned with matrix algebra.
> It is fine in the comments to use conventional mathematical notation, but I
> agree with Francois that the code should stick to VTK coding conventions
> wherever possible. Having comments inside of a //BTX //ETX block does not
> present any issues.
> I think the comments are a little on the verbose side, I would rather go
> into detail about how the matrix is laid out in memory in the class
> description, and then the individual function documentation can be much
> briefer. So explain row major order, assume the reader knows about matrix
> algebra in the class description and then keep brief function descriptions.
> How does that sound? I am hoping to add some more inline doxygen code
> examples to some of the new code I have added, examples of which can already
> be seen (not by me) in the OpenGLExtensionManager header.
> Marcus
> --
> Marcus D. Hanwell, Ph.D.
> R&D Engineer, Kitware Inc.
> (518) 881-4937
>

Marcus,

Thanks for the thoughtful response. I have a few responses/comments below:

1)
The comments can't use capital letters for matrices if the prototypes
do not follow suit. For example, this doesn't make sense:

  // Multiplies matrices A and B and stores the result in C according to
  // C = AB.
  static void Multiply3x3(vtkMatrix3x3 *a, vtkMatrix3x3 *b, vtkMatrix3x3 *c)

Because A,B,C are undefined to the user as a,b,c are the only things
that appear in the function (clearly it is trivial in this case, but
we are trying to create guidelines here).

My point once more is that this:

  // Multiplies matrices a and b and stores the result in c according to
  // c = ab.
  static void Multiply3x3(vtkMatrix3x3 *a, vtkMatrix3x3 *b, vtkMatrix3x3 *c)

doesn't "look like" a matrix multiplication, it "looks like" more of
an inner product of vectors or something, know what I mean. Again, in
this simple case it is not so important, but with other functions
where the user would not immediately know what is going on with the
implementation it would be helpful for the notation to match that
which he is used to.

2) I REALLY like the idea of little code snippets in the
documentation. Ideally, in EVERY class I think there should be
a) a little snippet with each non-trivial function
b) a link to a compilable example usage of the class (the things I've
been putting on the wiki)
c) a one line "Necessary libraries: vtkHybrid, vtkWidgets" that tells
the user which libraries they will need to link to in order to use
this class.

Thoughts on these 3 things? If you think these snippets/verbose
descriptions clutter the code (which they certainly would), should we
start a "VTK Software Guide" mimicking the "ITK Software Guide"? It
would be a big project to build and maintain, but I think it would be
invaluable. What would HAVE to happen is some real delegation and team
work of the whole community. I feel too often that we do not leverage
the huge numbers of VTK users for things like this.

3)
For most functions, the memory storage convention (row major) doesn't
matter - in fact, I thought this was the whole idea of providing a
Matrix3x3 class - why would anyone use a 9-vector to represent a
matrix? Should that even be supported? It seems like it is just asking
for trouble and there is already a good solution in place. That said,
if I describe this in the class documentation, can we then say for all
the function that take a double[9] something like "See the class
description for a description of the memory storage convention."?

The WORST case scenario is for the user to see the function and say
"Man this is annoying, I wonder which order the elements are supposed
to be in? I'm going to have to make a little demo and try it to make
sure I know what it is doing before I use it in my code". What it is
doing MUST be explained. The "little demo" MUST be provided. Moving
the responsibility of process from the user to the toolkit is pretty
much my whole motivation for this "documentation clean up" effort.

Thanks,

David